-
Notifications
You must be signed in to change notification settings - Fork 423
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
trace: split tracer
and versioned_tracer
methods
#682
Conversation
The [trace spec] requires that `TracerProvider`s MUST accept optional `version` and `schema_url` parameters. This introduces ergonomic issues as rust does not have a variadic solution outside of macros, leading to many calls with only a single relevant argument (e.g. `tracer(name, None, None)`). This patch splits the current `TracerProvider::tracer` method into `TracerProvider::versioned_tracer` which implements the spec mandated optional fields, as well as a `TracerProvider::tracer` method that accepts only a `name` parameter as a convenience method for the above listed common case. [trace spec]: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.4.0/specification/trace/api.md#get-a-tracer
Codecov Report
@@ Coverage Diff @@
## main #682 +/- ##
==========================================
- Coverage 63.85% 63.81% -0.05%
==========================================
Files 95 96 +1
Lines 7717 7725 +8
==========================================
+ Hits 4928 4930 +2
- Misses 2789 2795 +6
Continue to review full report at Codecov.
|
I think it's related to #555. Since we started preparing GA. One question here is what if the spec adds another parameter in the future? |
I think as the trace spec is frozen we can assume they will not for now. If they do, then the simplest thing seems to be to either add another method or do a major version bump to support the new spec. |
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.
Looks good 👍
I think we can close #555 with this PR |
The trace spec requires that
TracerProvider
s MUST accept optionalversion
andschema_url
parameters. This introduces ergonomic issues as rust does not have a variadic solution outside of macros, leading to many calls with only a single relevant argument (e.g.tracer(name, None, None)
).This patch splits the current
TracerProvider::tracer
method intoTracerProvider::versioned_tracer
which implements the spec mandated optional fields, as well as aTracerProvider::tracer
method that accepts only aname
parameter as a convenience method for the above listed common case.