Skip to content
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

Add autoprop package to provide configuration of propagators with useful defaults and envar support #2258

Merged
merged 28 commits into from
Jun 2, 2022

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented May 3, 2022

Use case

otel.SetTextMapPropagator(autoprop.NewTextMapPropagator())

to replace

otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{}))
  • Defaults to TraceContext with Baggage
  • Arguments to NewTextMapPropagator act the same as NewCompositeTextMapPropagator and override defaults
  • OTEL_PROPAGATORS overrides defaults and arguments
  • Custom 3rd-party propagators can be registered with RegisterTextMapPropagator and then supported in the parsed propagator from OTEL_PROPAGATORS

Resolve open-telemetry/opentelemetry-go#1265
Resolve open-telemetry/opentelemetry-go#2304
Related to open-telemetry/opentelemetry-go#1404

@codecov
Copy link

codecov bot commented May 3, 2022

Codecov Report

Merging #2258 (ded3cb7) into main (b7910af) will increase coverage by 0.3%.
The diff coverage is 100.0%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #2258     +/-   ##
=======================================
+ Coverage   73.7%   74.1%   +0.3%     
=======================================
  Files        142     144      +2     
  Lines       6457    6549     +92     
=======================================
+ Hits        4764    4856     +92     
  Misses      1550    1550             
  Partials     143     143             
Impacted Files Coverage Δ
propagators/autoprop/propagator.go 100.0% <100.0%> (ø)
propagators/autoprop/registry.go 100.0% <100.0%> (ø)

@pellared

This comment was marked as resolved.

@pellared

This comment was marked as resolved.

@MrAlias

This comment was marked as resolved.

@pellared

This comment was marked as resolved.

@MrAlias

This comment was marked as resolved.

@pellared

This comment was marked as resolved.

@pellared
Copy link
Member

pellared commented May 4, 2022

I had a call with @MrAlias and we discussed 👆

The current approach with autoprop is safer as

  1. We create less packages - less confusion for the users
  2. Having it in contrib allows to make breaking changes (as it is still v0)

propagators/autoprop/registry.go Outdated Show resolved Hide resolved
propagators/autoprop/propagator_test.go Outdated Show resolved Hide resolved
@MrAlias MrAlias requested a review from dmathieu as a code owner May 24, 2022 15:01
propagators/autoprop/example_test.go Show resolved Hide resolved
propagators/autoprop/propagator.go Show resolved Hide resolved
propagators/autoprop/registry.go Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support the OTEL_PROPAGATORS environment variable propagator configuration needs better shorthand
4 participants