-
Notifications
You must be signed in to change notification settings - Fork 226
feat: Add SQL Comment Propagator #1818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
arielvalentin
wants to merge
4
commits into
open-telemetry:main
Choose a base branch
from
arielvalentin:sql-comment-propagation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
c674b40
feat: Add SQL Comment Propagator
arielvalentin a0ff1a4
Merge branch 'main' into sql-comment-propagation
arielvalentin 762fca4
squash: use shift instead of concat for faster and more idiomatic ins…
arielvalentin c8b5ed5
squash: inline string generation
arielvalentin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
94 changes: 94 additions & 0 deletions
94
helpers/sql-processor/lib/opentelemetry/helpers/sql_processor/commenter.rb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| # Copyright The OpenTelemetry Authors | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| require 'cgi' | ||
|
|
||
| module OpenTelemetry | ||
| module Helpers | ||
| module SqlProcessor | ||
| # SqlCommenter provides SQL comment-based trace context propagation | ||
| # according to the SQL Commenter specification. | ||
| # | ||
| # This module implements a propagator interface compatible with Vitess, | ||
| # allowing it to be used as a drop-in replacement. | ||
| # | ||
| # @api public | ||
| module SqlCommenter | ||
| extend self | ||
|
|
||
| # SqlQuerySetter is responsible for formatting trace context as SQL comments | ||
| # and appending them to SQL queries according to the SQL Commenter specification. | ||
| # | ||
| # Format: /*key='value',key2='value2'*/ | ||
| # Values are URL-encoded per the SQL Commenter spec | ||
| module SqlQuerySetter | ||
| extend self | ||
|
|
||
| # Appends trace context as a SQL comment to the carrier (SQL query string) | ||
| # | ||
| # @param carrier [String] The SQL query string to modify | ||
| # @param headers [Hash] Hash of trace context headers (e.g., {'traceparent' => '00-...'}) | ||
| def set(carrier, headers) | ||
| return if headers.empty? | ||
| return if carrier.frozen? | ||
|
|
||
| # Convert headers hash to SQL commenter format | ||
| # Format: /*key1='value1',key2='value2'*/ | ||
| comment_parts = headers.map do |key, value| | ||
| # URL encode values as per SQL Commenter spec (using URI component encoding) | ||
| encoded_value = CGI.escapeURIComponent(value.to_s) | ||
| "#{key}='#{encoded_value}'" | ||
| end | ||
|
|
||
| comment = "/*#{comment_parts.join(',')}*/" | ||
|
|
||
| # Append to end of query (spec recommendation) | ||
| carrier.concat(" #{comment}") | ||
| end | ||
| end | ||
|
|
||
| # SqlQueryPropagator propagates trace context using SQL comments | ||
| # according to the SQL Commenter specification. | ||
| # | ||
| # This propagator implements the same interface as the Vitess propagator | ||
| # and can be used as a drop-in replacement. | ||
| # | ||
| # @example | ||
| # propagator = OpenTelemetry::Helpers::SqlProcessor::SqlCommenter.sql_query_propagator | ||
| # sql = "SELECT * FROM users" | ||
| # propagator.inject(sql, context: current_context) | ||
| # # => "SELECT * FROM users /*traceparent='00-...',tracestate='...'*/" | ||
| module SqlQueryPropagator | ||
| extend self | ||
|
|
||
| # Injects trace context into a SQL query as a comment | ||
| # | ||
| # @param carrier [String] The SQL query string to inject context into | ||
| # @param context [optional, Context] The context to inject. Defaults to current context. | ||
| # @param setter [optional, #set] The setter to use for appending the comment. | ||
| # Defaults to SqlQuerySetter. | ||
| # @return [nil] | ||
| def inject(carrier, context: OpenTelemetry::Context.current, setter: SqlQuerySetter) | ||
| # Use the global propagator to extract headers into a hash | ||
| headers = {} | ||
| OpenTelemetry.propagation.inject(headers, context: context) | ||
|
|
||
| # Pass the headers to our SQL comment setter | ||
| setter.set(carrier, headers) | ||
| nil | ||
| end | ||
| end | ||
|
|
||
| # Returns the SqlQueryPropagator module for stateless propagation | ||
| # | ||
| # @return [Module] The SqlQueryPropagator module | ||
| def sql_query_propagator | ||
| SqlQueryPropagator | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
120 changes: 120 additions & 0 deletions
120
helpers/sql-processor/test/opentelemetry/helpers/sql_processor/commenter_test.rb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| # Copyright The OpenTelemetry Authors | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| require 'test_helper' | ||
|
|
||
| describe OpenTelemetry::Helpers::SqlProcessor::SqlCommenter do | ||
| let(:span_id) { 'e457b5a2e4d86bd1' } | ||
| let(:trace_id) { '80f198ee56343ba864fe8b2a57d3eff7' } | ||
| let(:trace_flags) { OpenTelemetry::Trace::TraceFlags::SAMPLED } | ||
|
|
||
| let(:context) do | ||
| OpenTelemetry::Trace.context_with_span( | ||
| OpenTelemetry::Trace.non_recording_span( | ||
| OpenTelemetry::Trace::SpanContext.new( | ||
| trace_id: Array(trace_id).pack('H*'), | ||
| span_id: Array(span_id).pack('H*'), | ||
| trace_flags: trace_flags | ||
| ) | ||
| ) | ||
| ) | ||
| end | ||
|
|
||
| describe 'SqlQueryPropagator.inject' do | ||
| let(:propagator) { OpenTelemetry::Helpers::SqlProcessor::SqlCommenter.sql_query_propagator } | ||
|
|
||
| it 'injects trace context into SQL' do | ||
| sql = +'SELECT * FROM users' | ||
| propagator.inject(sql, context: context) | ||
|
|
||
| expected = "SELECT * FROM users /*traceparent='00-#{trace_id}-#{span_id}-01'*/" | ||
| _(sql).must_equal(expected) | ||
| end | ||
|
|
||
| it 'handles frozen strings by not modifying them' do | ||
| sql = -'SELECT * FROM users' | ||
| propagator.inject(sql, context: context) | ||
|
|
||
| # Frozen string should remain unchanged (setter will return early) | ||
| _(sql).must_equal('SELECT * FROM users') | ||
| end | ||
|
|
||
| it 'handles empty context' do | ||
| sql = +'SELECT * FROM users' | ||
| propagator.inject(sql, context: OpenTelemetry::Context.empty) | ||
|
|
||
| # Should not modify SQL when context produces no headers | ||
| _(sql).must_equal('SELECT * FROM users') | ||
| end | ||
|
|
||
| it 'includes tracestate when present' do | ||
| span_context = OpenTelemetry::Trace::SpanContext.new( | ||
| trace_id: Array(trace_id).pack('H*'), | ||
| span_id: Array(span_id).pack('H*'), | ||
| trace_flags: trace_flags, | ||
| tracestate: OpenTelemetry::Trace::Tracestate.from_string('congo=t61rcWkgMzE,rojo=00f067aa0ba902b7') | ||
| ) | ||
|
|
||
| ctx = OpenTelemetry::Trace.context_with_span( | ||
| OpenTelemetry::Trace.non_recording_span(span_context) | ||
| ) | ||
|
|
||
| sql = +'SELECT * FROM users' | ||
| propagator.inject(sql, context: ctx) | ||
|
|
||
| expected = "SELECT * FROM users /*traceparent='00-#{trace_id}-#{span_id}-01',tracestate='congo%3Dt61rcWkgMzE%2Crojo%3D00f067aa0ba902b7'*/" | ||
| _(sql).must_equal(expected) | ||
| end | ||
|
|
||
| it 'returns nil' do | ||
| sql = +'SELECT * FROM users' | ||
| result = propagator.inject(sql, context: context) | ||
|
|
||
| _(result).must_be_nil | ||
| end | ||
| end | ||
|
|
||
| describe 'SqlQuerySetter.set' do | ||
| let(:setter) { OpenTelemetry::Helpers::SqlProcessor::SqlCommenter::SqlQuerySetter } | ||
|
|
||
| it 'formats headers as SQL comments' do | ||
| sql = +'SELECT * FROM users' | ||
| headers = { 'traceparent' => '00-80f198ee56343ba864fe8b2a57d3eff7-e457b5a2e4d86bd1-01' } | ||
|
|
||
| setter.set(sql, headers) | ||
|
|
||
| expected = "SELECT * FROM users /*traceparent='00-80f198ee56343ba864fe8b2a57d3eff7-e457b5a2e4d86bd1-01'*/" | ||
| _(sql).must_equal(expected) | ||
| end | ||
|
|
||
| it 'URL encodes values' do | ||
| sql = +'SELECT * FROM users' | ||
| headers = { 'key' => 'value with spaces' } | ||
|
|
||
| setter.set(sql, headers) | ||
|
|
||
| expected = "SELECT * FROM users /*key='value%20with%20spaces'*/" | ||
| _(sql).must_equal(expected) | ||
| end | ||
|
|
||
| it 'handles empty headers' do | ||
| sql = +'SELECT * FROM users' | ||
| setter.set(sql, {}) | ||
|
|
||
| _(sql).must_equal('SELECT * FROM users') | ||
| end | ||
|
|
||
| it 'handles frozen strings by not modifying them' do | ||
| sql = -'SELECT * FROM users' | ||
| headers = { 'traceparent' => '00-80f198ee56343ba864fe8b2a57d3eff7-e457b5a2e4d86bd1-01' } | ||
|
|
||
| setter.set(sql, headers) | ||
|
|
||
| # Frozen string should remain unchanged | ||
| _(sql).must_equal('SELECT * FROM users') | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated but I am fixing a missing new line at the end of this file