-
Notifications
You must be signed in to change notification settings - Fork 197
feat: http semconv opt in files #1547
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
feat: http semconv opt in files #1547
Conversation
::HTTP::Connection.prepend(Patches::Connection) | ||
def determine_semconv | ||
case ENV.fetch('OTEL_SEMCONV_STABILITY_OPT_IN', nil) | ||
when 'http/dup' |
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.
http/dup
takes precedence, so we check this first.
'server.address' => uri.host, | ||
'server.port' => uri.port | ||
} | ||
attributes['url.query'] = uri.query unless uri.query.nil? |
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.
Queries weren't previously reported and can often be nil
, so we check/add this attribute separately.
|
||
semconv_stability = %w[dup stable old] | ||
|
||
semconv_stability.each do |mode| |
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.
This approach gives us a fresh HTTP instance on each run. Without this, a single HTTP instance receives all patches.
…amadan/opentelemetry-ruby-contrib into http_semcon_opt_in_files
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.
I love the simplicity of this approach, especially since we're only required to keep it for six months. Migrating to the stable semantic conventions also makes metrics integration much easier. Thank you!
We're planning to merge this on Monday, June 16 if there are no blocking comments. |
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.
GitHub
This PR is intended to assist in the transition from the old to new HTTP semantic conventions. Per the HTTP semantic convention stability migration spec, users should be able to set the environment variable
OTEL_SEMCONV_STABILITY_OPT_IN
to:http
to emit stable conventions onlyhttp/dup
to emit both old and the stable conventionsThe agent is required to maintain this bridge for 6 months and may drop the environment variable in the next major version and emit only the stable HTTP and networking conventions.
While this approach introduces multiple new and similar files and tests (potentially making debugging and maintenance more difficult), the tradeoff ensures we're only patching the HTTP class once and makes the eventual cleanup of the old conventions mostly a matter of deleting files and small refactors. Another approach (also taken by Python) introduces a
StabilityMode
class that checks calls to determine which attributes to send, however this approach adds complexity of calling a method every time an attribute is added.Hopefully with the temporary nature of this work, this solution can be seen as a way to start the transition. The approach can also be easily duplicated to other HTTP libraries and support the similar DB migration.