associated type Span, Tracer as short-hand, and *Protocol types #93
Conversation
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
**Motivation:** This is a revival of #84 where we try to KEEP compatibility with versions below 5.7 with a compatibility "legacy" tracer type, but otherwise move towards requiring 5.7 for all the "nice" apis that use associated types and `any TracerProtocol` and friends **Modifications:** - `Tracer` -> `TracerProtocol` - `Tracer` is now a namespace in order to `Tracer.withSpan {}` easily - `Span` -> `SpanProtocol` - Introduce `LegacyTracerProtocol` which does not make use of associated type Span, and can be used in 5.6 libraries; they can deprecate and move away form it ASAP as they start requiring 5.7 **Result:** Offer the APIs we want in 5.7 but remain compatible with 5.6 until we drop it as soon as 5.9 is released as stable - this allows us to adopt eagerly in libraries without having to wait for 5.9 to drop.
ktoso
force-pushed
the
wip-towards-5.7-2
branch
from
9d7a9c8 to
afb07cf
Compare
stevapple
approved these changes
Mar 15, 2023
README.md
Outdated
| @@ -261,7 +261,7 @@ When instrumenting server applications there are typically three parties involve | |||
|
|
|||
| 1. [Application developers](#application-developers-setting-up-instruments) creating server-side applications | |||
| 2. [Library/Framework developers](#libraryframework-developers-instrumenting-your-software) providing building blocks to create these applications | |||
| 3. [Instrument developers](#instrument-developers-creating-an-instrument) providing tools to collect distributed metadata about your application | |||
| 3. [InstrumentProtocol developers](#instrument-developers-creating-an-instrument) providing tools to collect distributed metadata about your application | |||
There was a problem hiding this comment.
Ah, a batch rename mistake?
There was a problem hiding this comment.
Thanks yeah will clean it up
|
Pinged some folks for more review -- if you spot anything off please comment and I'll address in follow ups! 👍 |
|
Resolves #78 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation:
This is a revival of #84
where we try to KEEP compatibility with versions below 5.7 with a
compatibility "legacy" tracer type, but otherwise move towards requiring
5.7 for all the "nice" apis that use associated types and
any TracerProtocoland friendsReplaces #92
Modifications:
Tracer->TracerProtocolTraceris now a namespace in order toTracer.withSpan {}easilySpan->SpanProtocolLegacyTracerProtocolwhich does not make use of associatedtype Span, and can be used in 5.6 libraries; they can deprecate and
move away form it ASAP as they start requiring 5.7
Result:
Offer the APIs we want in 5.7 but remain compatible with 5.6 until we
drop it as soon as 5.9 is released as stable - this allows us to adopt
eagerly in libraries without having to wait for 5.9 to drop.