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

Fix issue -propagation type is not case insensitive #483

Merged
merged 6 commits into from
Jul 6, 2022

Conversation

xyq175com
Copy link
Contributor

@xyq175com xyq175com commented Jul 5, 2022

Make the context propagation extraction case insensitive. See link:#3149

Changes

Do a case-insensitive comparison in the Grpc.Core instrumentation

Fix B3 propagator is not case insensitive #3149
correct white space formatting
@xyq175com xyq175com requested a review from a team as a code owner July 5, 2022 04:25
@xyq175com xyq175com changed the title Fix issue 3149 Fix issue -B3 propagator is not case insensitive Jul 5, 2022
@pellared
Copy link
Member

pellared commented Jul 5, 2022

  1. Can you mention this in src/OpenTelemetry.Instrumentation.GrpcCore/CHANGELOG.md?
  2. I think this is about any propagation type - not just B3.
  3. Can you try adding a unit test for it to avoid a regression bug?

@xyq175com xyq175com changed the title Fix issue -B3 propagator is not case insensitive Fix issue -propagator is not case insensitive Jul 5, 2022
@xyq175com xyq175com changed the title Fix issue -propagator is not case insensitive Fix issue -propagation type is not case insensitive Jul 5, 2022
@xyq175com
Copy link
Contributor Author

  1. Can you mention this in src/OpenTelemetry.Instrumentation.GrpcCore/CHANGELOG.md?
  2. I think this is about any propagation type - not just B3.
  3. Can you try adding a unit test for it to avoid a regression bug?

Thank you I updated 'src/OpenTelemetry.Instrumentation.GrpcCore/CHANGELOG.md' and also tested the unit testing for this file and no failure.
image

@pellared
Copy link
Member

pellared commented Jul 5, 2022

also tested the unit testing for this file

I have written "try adding a new unit test" 😉

Co-authored-by: Robert Pająk <pellared@hotmail.com>
Co-authored-by: Robert Pająk <pellared@hotmail.com>
@utpilla utpilla added the comp:instrumentation.grpccore Things related to OpenTelemetry.Instrumentation.GrpcCore label Jul 5, 2022
@utpilla
Copy link
Contributor

utpilla commented Jul 5, 2022

Adding @pcwiese who the component owner. Could you please review this PR?

@codecov
Copy link

codecov bot commented Jul 5, 2022

Codecov Report

Merging #483 (523b7a7) into main (c55fd8d) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #483   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files        163     163           
  Lines       4979    4979           
=====================================
  Misses      4979    4979           
Impacted Files Coverage Δ
...strumentation.GrpcCore/ServerTracingInterceptor.cs 0.00% <0.00%> (ø)

@pcwiese
Copy link
Contributor

pcwiese commented Jul 6, 2022 via email

@utpilla
Copy link
Contributor

utpilla commented Jul 6, 2022

@xyq175com Could you please update the branch with the latest changes from main?

@xyq175com
Copy link
Contributor Author

@pcwiese I think you have the permission to approve the PR, please help on this, thank you.

@utpilla utpilla merged commit 73a0990 into open-telemetry:main Jul 6, 2022
@utpilla
Copy link
Contributor

utpilla commented Jul 6, 2022

@xyq175com Please follow the steps mentioned here if you want a new version of this package released: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/main/CONTRIBUTING.md#how-to-request-for-release-of-package

@xyq175com xyq175com deleted the Fix-issue-3149 branch July 6, 2022 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:instrumentation.grpccore Things related to OpenTelemetry.Instrumentation.GrpcCore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants