Adding Context implemenations for HeaderMap and MetadataMap

[swilcox3]

To make it easier to use opentelemetry from hyper and tonic, I added implementations for api::context::propagation::Carrier for HeaderMap and MetadataMap under feature flags http_context and tonic_context respectively.

cargo test --all passes, cargo clippy --all passes. Output in Jaeger of grpc example is the same. Output of cargo bench:

start-end-span/always-sample
                        time:   [1.3808 us 1.4051 us 1.4427 us]
Found 8 outliers among 100 measurements (8.00%)
  1 (1.00%) high mild
  7 (7.00%) high severe
start-end-span/never-sample
                        time:   [342.14 ns 356.66 ns 379.42 ns]
Found 7 outliers among 100 measurements (7.00%)
  3 (3.00%) high mild
  4 (4.00%) high severe

start-end-span-4-attrs/always-sample
                        time:   [3.0217 us 3.0788 us 3.1451 us]
Found 20 outliers among 100 measurements (20.00%)
  3 (3.00%) high mild
  17 (17.00%) high severe
start-end-span-4-attrs/never-sample
                        time:   [463.06 ns 504.61 ns 568.48 ns]
Found 9 outliers among 100 measurements (9.00%)
  2 (2.00%) high mild
  7 (7.00%) high severe

start-end-span-8-attrs/always-sample
                        time:   [4.9835 us 5.0433 us 5.1100 us]
Found 20 outliers among 100 measurements (20.00%)
  15 (15.00%) high mild
  5 (5.00%) high severe
start-end-span-8-attrs/never-sample
                        time:   [550.82 ns 577.73 ns 626.42 ns]
  5 (5.00%) high mild
  6 (6.00%) high severe

start-end-span-all-attr-types/always-sample
                        time:   [3.6182 us 3.6591 us 3.7112 us]
Found 13 outliers among 100 measurements (13.00%)
  2 (2.00%) low mild
  1 (1.00%) high mild
  10 (10.00%) high severe
start-end-span-all-attr-types/never-sample
                        time:   [521.50 ns 537.08 ns 563.59 ns]
Found 8 outliers among 100 measurements (8.00%)
  2 (2.00%) high mild
  6 (6.00%) high severe

start-end-span-all-attr-types-2x/always-sample
                        time:   [6.2272 us 6.2923 us 6.3647 us]
Found 11 outliers among 100 measurements (11.00%)
  9 (9.00%) high mild
  2 (2.00%) high severe
start-end-span-all-attr-types-2x/never-sample
                        time:   [693.60 ns 711.69 ns 751.74 ns]
Found 9 outliers among 100 measurements (9.00%)
  4 (4.00%) high mild
  5 (5.00%) high severe

[jtescher]

Nice start, other than the few lint errors seems like it would be nice to still be able to pass a static str to a carrier as well. Maybe have the methods take Into<String>? or could require fewer allocations by storing a Cow<'static str>?

[swilcox3]

Well, the trait itself does still take a &str. I just changed the prior implementation to HashMap<String, String>, because that's only used in test code.

[jtescher]

Yeah seems fine as it's not intended to be used in production as a carrier.

Also for the feature names, are you thinking that there would be other http/tonic features besides context? or would http and tonic be sufficient.

[swilcox3]

The feature names can't be the same as the dependency names, so that's why I put the _context suffix on them. I'm open to whatever naming scheme you want for those. It does seem conceivable that you could want other integrations with those crates besides just the context. Those are probably the two most common libraries people will integrate with. It'd make more sense if all of those integrations were under a single feature flag for each library, though.

[jtescher]

The crate name for optional dependencies is the feature name by default, so you can simply remove them from the feature list and it will be http and tonic.

[swilcox3]

Oh, you learn something new every day. I'll fix that.

[swilcox3]

Done.

[jtescher]

Looks good, thanks! Just have to sign the cla

[swilcox3]

OK done. Do I need to make another commit to make it run the check again?

[MikeGoldsmith]

Pushing a new change will re-trigger. It looks like some other CI tasks failed though.

[jtescher]

Looks like there is an issue with the nightly builds error: component 'rustfmt' for target 'x86_64-unknown-linux-gnu' is unavailable for download for channel nightly Sometimes not all components are available in any given nightly.. Can look into it.

[swilcox3]

@jtescher @MikeGoldsmith I'm not able to merge because the base branch is protected. Can you merge this please?

[jtescher]

@iredelmeier any way we can make the nightly checks optional? or get admin to merge anyway?

[iredelmeier]

@jtescher making the checks either optional or skipping them if a necessary component isn't available seems reasonable to me. (Giving admins permission to merge could also be viable but might be too slow. I don't think "maintainers" count as "admins" for that purpose, although I could be wrong.)

The lint script currently does what I mean above re: only actually running clippy if it's available - see here.

[jtescher]

@iredelmeier Updated the config in #120 but the previous checks are still marked as required in the protected branch config I believe (which I do not have access to).

[iredelmeier]

@iredelmeier Updated the config in #120 but the previous checks are still marked as required in the protected branch config I believe (which I do not have access to).

Moving the discussion over there :)

[jtescher]

@swilcox3 looks like rustfmt on nightly works again, just one lint left 🎉