diff options
-rw-r--r-- | jstests/telemetry/redact_queries_with_nonobject_fields.js | 7 | ||||
-rw-r--r-- | jstests/telemetry/telemetry_metrics_across_getMore_calls.js | 20 | ||||
-rw-r--r-- | src/mongo/db/pipeline/document_source_telemetry.cpp | 9 | ||||
-rw-r--r-- | src/mongo/db/query/query_feature_flags.idl | 5 | ||||
-rw-r--r-- | src/mongo/db/query/telemetry.cpp | 43 | ||||
-rw-r--r-- | src/mongo/db/query/telemetry.h | 4 | ||||
-rw-r--r-- | src/mongo/db/query/telemetry_store_test.cpp | 8 |
7 files changed, 77 insertions, 19 deletions
diff --git a/jstests/telemetry/redact_queries_with_nonobject_fields.js b/jstests/telemetry/redact_queries_with_nonobject_fields.js index 8bc7aad5a89..b46c38f8146 100644 --- a/jstests/telemetry/redact_queries_with_nonobject_fields.js +++ b/jstests/telemetry/redact_queries_with_nonobject_fields.js @@ -2,13 +2,18 @@ * Test that telemetry key generation works for queries with non-object fields. */ load('jstests/libs/analyze_plan.js'); +load("jstests/libs/feature_flag_util.js"); (function() { "use strict"; +if (!FeatureFlagUtil.isEnabled(db, "Telemetry")) { + return; +} + // Turn on the collecting of telemetry metrics. let options = { - setParameter: "internalQueryConfigureTelemetrySamplingRate=2147483647", + setParameter: {internalQueryConfigureTelemetrySamplingRate: 2147483647}, }; const conn = MongoRunner.runMongod(options); diff --git a/jstests/telemetry/telemetry_metrics_across_getMore_calls.js b/jstests/telemetry/telemetry_metrics_across_getMore_calls.js index c36bc61604c..85d4f709220 100644 --- a/jstests/telemetry/telemetry_metrics_across_getMore_calls.js +++ b/jstests/telemetry/telemetry_metrics_across_getMore_calls.js @@ -1,15 +1,18 @@ /** * Test that the telemetry metrics are updated correctly across getMores. */ -load('jstests/libs/analyze_plan.js'); load("jstests/libs/profiler.js"); // For getLatestProfilerEntry. (function() { "use strict"; -// Turn on the collection of telemetry metrics. +if (!FeatureFlagUtil.isEnabled(db, "Telemetry")) { + return; +} + +// Turn on the collecting of telemetry metrics. let options = { - setParameter: "internalQueryConfigureTelemetrySamplingRate=2147483647", + setParameter: {internalQueryConfigureTelemetrySamplingRate: 2147483647}, }; const conn = MongoRunner.runMongod(options); @@ -18,6 +21,10 @@ var coll = testDB[jsTestName()]; var collTwo = db[jsTestName() + 'Two']; coll.drop(); +// Make it easier to extract correct telemetry store entry for purposes of this test. +assert.commandWorked(testDB.adminCommand( + {setParameter: 1, internalQueryConfigureTelemetryFieldNameRedactionStrategy: "none"})); + function verifyMetrics(batch) { batch.forEach(element => { assert(element.metrics.docsScanned.sum > element.metrics.docsScanned.min); @@ -73,11 +80,14 @@ verifyMetrics(telStore.cursor.firstBatch); // Ensure that for queries using an index, keys scanned is nonzero. assert.commandWorked(coll.createIndex({bar: 1})); -coll.aggregate([{$match: {bar: 1}}], {cursor: {batchSize: 2}}); +coll.aggregate([{$match: {$or: [{bar: 1, foo: 1}]}}], {cursor: {batchSize: 2}}); // This filters telemetry entries to just the one entered for the above agg command. telStore = testDB.adminCommand({ aggregate: 1, - pipeline: [{$telemetry: {}}, {$match: {"key.pipeline.$match.bar": {$eq: "###"}}}], + pipeline: [ + {$telemetry: {}}, + {$match: {"key.pipeline.$match.$or": {$eq: [{'bar': '###', 'foo': '###'}]}}} + ], cursor: {} }); assert(telStore.cursor.firstBatch[0].metrics.keysScanned.sum > 0); diff --git a/src/mongo/db/pipeline/document_source_telemetry.cpp b/src/mongo/db/pipeline/document_source_telemetry.cpp index 812261edc1d..b08adf97853 100644 --- a/src/mongo/db/pipeline/document_source_telemetry.cpp +++ b/src/mongo/db/pipeline/document_source_telemetry.cpp @@ -34,10 +34,11 @@ namespace mongo { -REGISTER_DOCUMENT_SOURCE(telemetry, - DocumentSourceTelemetry::LiteParsed::parse, - DocumentSourceTelemetry::createFromBson, - AllowedWithApiStrict::kNeverInVersion1); +REGISTER_DOCUMENT_SOURCE_WITH_FEATURE_FLAG(telemetry, + DocumentSourceTelemetry::LiteParsed::parse, + DocumentSourceTelemetry::createFromBson, + AllowedWithApiStrict::kNeverInVersion1, + feature_flags::gFeatureFlagTelemetry); std::unique_ptr<DocumentSourceTelemetry::LiteParsed> DocumentSourceTelemetry::LiteParsed::parse( const NamespaceString& nss, const BSONElement& spec) { diff --git a/src/mongo/db/query/query_feature_flags.idl b/src/mongo/db/query/query_feature_flags.idl index be524382d81..e053c5080cf 100644 --- a/src/mongo/db/query/query_feature_flags.idl +++ b/src/mongo/db/query/query_feature_flags.idl @@ -80,3 +80,8 @@ feature_flags: of queries, including NLJ $lookup plans. Also enables the SBE plan cache." cpp_varname: gFeatureFlagSbeFull default: false + + featureFlagTelemetry: + description: "Feature flag for enabling the telemetry store." + cpp_varname: gFeatureFlagTelemetry + default: false
\ No newline at end of file diff --git a/src/mongo/db/query/telemetry.cpp b/src/mongo/db/query/telemetry.cpp index 05b9a46b351..90e7da76ddb 100644 --- a/src/mongo/db/query/telemetry.cpp +++ b/src/mongo/db/query/telemetry.cpp @@ -38,11 +38,13 @@ #include "mongo/db/pipeline/aggregate_command_gen.h" #include "mongo/db/query/find_command_gen.h" #include "mongo/db/query/plan_explainer.h" +#include "mongo/db/query/query_feature_flags_gen.h" #include "mongo/db/query/query_planner_params.h" #include "mongo/db/query/rate_limiting.h" #include "mongo/db/query/telemetry_util.h" #include "mongo/logv2/log.h" #include "mongo/rpc/metadata/client_metadata.h" +#include "mongo/util/assert_util.h" #include "mongo/util/system_clock_source.h" #include <cstddef> @@ -52,6 +54,11 @@ namespace mongo { namespace telemetry { +bool isTelemetryEnabled() { + return feature_flags::gFeatureFlagTelemetry.isEnabledAndIgnoreFCV(); +} + + namespace { /** @@ -135,6 +142,13 @@ const auto telemetryRateLimiter = ServiceContext::ConstructorActionRegisterer telemetryStoreManagerRegisterer{ "TelemetryStoreManagerRegisterer", [](ServiceContext* serviceCtx) { + if (!isTelemetryEnabled()) { + // featureFlags are not allowed to be changed at runtime. Therefore it's not an issue + // to not create a telemetry store in ConstructorActionRegisterer at start up with the + // flag off - because the flag can not be turned on at any point afterwards. + return; + } + telemetry_util::telemetryStoreOnParamChangeUpdater(serviceCtx) = std::make_unique<TelemetryOnParamChangeUpdaterImpl>(); auto status = memory_util::MemorySize::parse(queryTelemetryStoreSize.get()); @@ -167,17 +181,17 @@ ServiceContext::ConstructorActionRegisterer telemetryStoreManagerRegisterer{ std::make_unique<RateLimiting>(queryTelemetrySamplingRate.load()); }}; -bool isTelemetryEnabled(const ServiceContext* serviceCtx) { - return telemetryRateLimiter(serviceCtx)->getSamplingRate() > 0; -} - /** * Internal check for whether we should collect metrics. This checks the rate limiting * configuration for a global on/off decision and, if enabled, delegates to the rate limiter. */ bool shouldCollect(const ServiceContext* serviceCtx) { // Quick escape if telemetry is turned off. - if (!isTelemetryEnabled(serviceCtx)) { + if (!isTelemetryEnabled()) { + return false; + } + // Cannot collect telemetry if sampling rate is not greater than 0. + if (telemetryRateLimiter(serviceCtx)->getSamplingRate() <= 0) { return false; } // Check if rate limiting allows us to collect telemetry for this request. @@ -383,6 +397,11 @@ const BSONObj& TelemetryMetrics::redactKey(const BSONObj& key) const { } void registerAggRequest(const AggregateCommandRequest& request, OperationContext* opCtx) { + + if (!isTelemetryEnabled()) { + return; + } + if (request.getEncryptionInformation()) { return; } @@ -436,6 +455,9 @@ void registerAggRequest(const AggregateCommandRequest& request, OperationContext void registerFindRequest(const FindCommandRequest& request, const NamespaceString& collection, OperationContext* opCtx) { + if (!isTelemetryEnabled()) { + return; + } if (request.getEncryptionInformation()) { return; } @@ -472,6 +494,9 @@ void registerFindRequest(const FindCommandRequest& request, } void registerGetMoreRequest(OperationContext* opCtx, const PlanExplainer& planExplainer) { + if (!isTelemetryEnabled()) { + return; + } auto&& telemetryKey = planExplainer.getTelemetryKey(); if (telemetryKey.isEmpty() || !shouldCollect(opCtx->getServiceContext())) { return; @@ -481,14 +506,20 @@ void registerGetMoreRequest(OperationContext* opCtx, const PlanExplainer& planEx std::pair<TelemetryStore*, Lock::ResourceLock> getTelemetryStoreForRead( const ServiceContext* serviceCtx) { + uassert(6579000, "Telemetry is not enabled without the feature flag on", isTelemetryEnabled()); return telemetryStoreDecoration(serviceCtx)->getTelemetryStore(); } std::unique_ptr<TelemetryStore> resetTelemetryStore(const ServiceContext* serviceCtx) { + uassert(6579002, "Telemetry is not enabled without the feature flag on", isTelemetryEnabled()); return telemetryStoreDecoration(serviceCtx)->resetTelemetryStore(); } -void recordExecution(const OperationContext* opCtx, const OpDebug& opDebug, bool isFle) { +void recordExecution(OperationContext* opCtx, const OpDebug& opDebug, bool isFle) { + + if (!isTelemetryEnabled()) { + return; + } if (isFle) { return; } diff --git a/src/mongo/db/query/telemetry.h b/src/mongo/db/query/telemetry.h index 0e225d2f17e..1d357570774 100644 --- a/src/mongo/db/query/telemetry.h +++ b/src/mongo/db/query/telemetry.h @@ -177,8 +177,6 @@ std::pair<TelemetryStore*, Lock::ResourceLock> getTelemetryStoreForRead( std::unique_ptr<TelemetryStore> resetTelemetryStore(const ServiceContext* serviceCtx); -bool isTelemetryEnabled(const ServiceContext* serviceCtx); - /** * Register a request for telemetry collection. The telemetry machinery may decide not to collect * anything but this should be called for all requests. The decision is made based on the feature @@ -198,7 +196,7 @@ void registerFindRequest(const FindCommandRequest& request, void registerGetMoreRequest(OperationContext* opCtx, const PlanExplainer& planExplainer); -void recordExecution(const OperationContext* opCtx, const OpDebug& opDebug, bool isFle); +void recordExecution(OperationContext* opCtx, const OpDebug& opDebug, bool isFle); /** * Collect telemetry for the operation identified by `key`. The `isExec` flag should be set if it's diff --git a/src/mongo/db/query/telemetry_store_test.cpp b/src/mongo/db/query/telemetry_store_test.cpp index c05a6fb0aac..7845ac9267a 100644 --- a/src/mongo/db/query/telemetry_store_test.cpp +++ b/src/mongo/db/query/telemetry_store_test.cpp @@ -29,6 +29,7 @@ #include "mongo/bson/simple_bsonobj_comparator.h" #include "mongo/db/catalog/rename_collection.h" +#include "mongo/db/query/query_feature_flags_gen.h" #include "mongo/db/query/telemetry.h" #include "mongo/unittest/unittest.h" @@ -42,6 +43,13 @@ protected: }; TEST_F(TelemetryStoreTest, BasicUsage) { + // Turning on the flag at runtime will crash as telemetry store registerer (which creates the + // telemetry store) is called at start up and if flag is off, the telemetry store will have + // never been created. Thus, instead of turning on the flag at runtime and crashing, we skip the + // test if telemetry feature flag is off. + if (!feature_flags::gFeatureFlagTelemetry.isEnabledAndIgnoreFCV()) { + return; + } TelemetryStore telStore{5000000, 1000}; auto getMetrics = [&](BSONObj& key) { |