diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb index eed7959a2f1e522eb1886f6d71e85b3a97edd3f7..0c7195c5be37b65372390441c1cbb6a88467ea29 100644 --- a/app/graphql/gitlab_schema.rb +++ b/app/graphql/gitlab_schema.rb @@ -15,9 +15,6 @@ class GitlabSchema < GraphQL::Schema use Gitlab::Graphql::Tracers::MetricsTracer use Gitlab::Graphql::Tracers::LoggerTracer - # TODO: Old tracer which will be removed eventually - # See https://gitlab.com/gitlab-org/gitlab/-/issues/345396 - use Gitlab::Graphql::GenericTracing use Gitlab::Graphql::Tracers::TimerTracer use Gitlab::Graphql::Subscriptions::ActionCableWithLoadBalancing diff --git a/config/feature_flags/development/graphql_generic_tracing_metrics_deactivate.yml b/config/feature_flags/development/graphql_generic_tracing_metrics_deactivate.yml deleted file mode 100644 index c2954e791d61105de274da9aa748185415931c91..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/graphql_generic_tracing_metrics_deactivate.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: graphql_generic_tracing_metrics_deactivate -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123228 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/415004 -milestone: '16.1' -type: development -group: group::application performance -default_enabled: false diff --git a/lib/gitlab/graphql/generic_tracing.rb b/lib/gitlab/graphql/generic_tracing.rb deleted file mode 100644 index dc3f65746311829da8b541727805f8bc21a195d4..0000000000000000000000000000000000000000 --- a/lib/gitlab/graphql/generic_tracing.rb +++ /dev/null @@ -1,71 +0,0 @@ -# frozen_string_literal: true - -# This class is used as a hook to observe graphql runtime events. From this -# hook both gitlab metrics and opentracking measurements are generated - -module Gitlab - module Graphql - class GenericTracing < GraphQL::Tracing::PlatformTracing - self.platform_keys = { - 'lex' => 'graphql.lex', - 'parse' => 'graphql.parse', - 'validate' => 'graphql.validate', - 'analyze_query' => 'graphql.analyze', - 'analyze_multiplex' => 'graphql.analyze', - 'execute_multiplex' => 'graphql.execute', - 'execute_query' => 'graphql.execute', - 'execute_query_lazy' => 'graphql.execute', - 'execute_field' => 'graphql.execute', - 'execute_field_lazy' => 'graphql.execute' - } - - def platform_field_key(type, field) - "#{type.name}.#{field.name}" - end - - def platform_authorized_key(type) - "#{type.graphql_name}.authorized" - end - - def platform_resolve_type_key(type) - "#{type.graphql_name}.resolve_type" - end - - def platform_trace(platform_key, key, data, &block) - tags = { platform_key: platform_key, key: key } - start = Gitlab::Metrics::System.monotonic_time - - with_labkit_tracing(tags, &block) - ensure - duration = Gitlab::Metrics::System.monotonic_time - start - - graphql_duration_seconds.observe(tags, duration) unless deactivated? - end - - private - - def deactivated? - Feature.enabled?(:graphql_generic_tracing_metrics_deactivate) - end - - def with_labkit_tracing(tags, &block) - return yield unless Labkit::Tracing.enabled? - - name = "#{tags[:platform_key]}.#{tags[:key]}" - span_tags = { - 'component' => 'web', - 'span.kind' => 'server' - }.merge(tags.stringify_keys) - - Labkit::Tracing.with_tracing(operation_name: name, tags: span_tags, &block) - end - - def graphql_duration_seconds - @graphql_duration_seconds ||= Gitlab::Metrics.histogram( - :graphql_duration_seconds, - 'GraphQL execution time' - ) - end - end - end -end diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index b5c2d4da9ac734f30f793d9bd42d9e21feda388f..2e0711fe18c10d5cf1e1fc2c5904e4be8dfa54af 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -12,10 +12,6 @@ expect(tracers).to include(BatchLoader::GraphQL) end - it 'enables the generic instrumenter' do - expect(tracers).to include(instance_of(::Gitlab::Graphql::GenericTracing)) - end - it 'has the base mutation' do expect(described_class.mutation).to eq(::Types::MutationType) end diff --git a/spec/lib/gitlab/graphql/generic_tracing_spec.rb b/spec/lib/gitlab/graphql/generic_tracing_spec.rb deleted file mode 100644 index 04fe7760f62d5b0f013890716fdf213a50a83345..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/graphql/generic_tracing_spec.rb +++ /dev/null @@ -1,89 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Graphql::GenericTracing, feature_category: :application_performance do - let(:graphql_duration_seconds_histogram) { double('Gitlab::Metrics::NullMetric') } - - context 'when graphql_generic_tracing_metrics_deactivate is disabled' do - before do - stub_feature_flags(graphql_generic_tracing_metrics_deactivate: false) - end - - it 'updates graphql histogram with expected labels' do - query = 'query { users { id } }' - tracer = described_class.new - - allow(tracer) - .to receive(:graphql_duration_seconds) - .and_return(graphql_duration_seconds_histogram) - - expect_metric('graphql.lex', 'lex') - expect_metric('graphql.parse', 'parse') - expect_metric('graphql.validate', 'validate') - expect_metric('graphql.analyze', 'analyze_multiplex') - expect_metric('graphql.execute', 'execute_query_lazy') - expect_metric('graphql.execute', 'execute_multiplex') - - GitlabSchema.execute(query, context: { tracers: [tracer] }) - end - end - - context 'when graphql_generic_tracing_metrics_deactivate is enabled' do - it 'does not updates graphql histogram with expected labels' do - query = 'query { users { id } }' - tracer = described_class.new - - allow(tracer) - .to receive(:graphql_duration_seconds) - .and_return(graphql_duration_seconds_histogram) - - GitlabSchema.execute(query, context: { tracers: [tracer] }) - - expect(graphql_duration_seconds_histogram) - .not_to receive(:observe) - end - end - - context "when labkit tracing is enabled" do - before do - expect(Labkit::Tracing).to receive(:enabled?).and_return(true) - end - - it 'yields with labkit tracing' do - expected_tags = { - 'component' => 'web', - 'span.kind' => 'server', - 'platform_key' => 'pkey', - 'key' => 'key' - } - - expect(Labkit::Tracing) - .to receive(:with_tracing) - .with(operation_name: "pkey.key", tags: expected_tags) - .and_yield - - expect { |b| described_class.new.platform_trace('pkey', 'key', nil, &b) }.to yield_control - end - end - - context "when labkit tracing is disabled" do - before do - expect(Labkit::Tracing).to receive(:enabled?).and_return(false) - end - - it 'yields without measurement' do - expect(Labkit::Tracing).not_to receive(:with_tracing) - - expect { |b| described_class.new.platform_trace('pkey', 'key', nil, &b) }.to yield_control - end - end - - private - - def expect_metric(platform_key, key) - expect(graphql_duration_seconds_histogram) - .to receive(:observe) - .with({ platform_key: platform_key, key: key }, be > 0.0) - end -end