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

Adds possibility to set headers to zipkin exporter #1202

Merged
merged 7 commits into from
Jun 18, 2020

Conversation

obecny
Copy link
Member

@obecny obecny commented Jun 16, 2020

Which problem is this PR solving?

Short description of the changes

  • It adds possibility to set headers to zipkin exporter

@obecny obecny self-assigned this Jun 16, 2020
@obecny obecny added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. enhancement New feature or request labels Jun 16, 2020
@codecov
Copy link

codecov bot commented Jun 16, 2020

Codecov Report

Merging #1202 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1202   +/-   ##
=======================================
  Coverage   92.32%   92.32%           
=======================================
  Files         122      122           
  Lines        3533     3533           
  Branches      714      714           
=======================================
  Hits         3262     3262           
  Misses        271      271           
Impacted Files Coverage Δ
...ackages/opentelemetry-exporter-zipkin/src/types.ts 100.00% <ø> (ø)
...ckages/opentelemetry-exporter-zipkin/src/zipkin.ts 98.46% <ø> (ø)

Copy link
Member

@mwear mwear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mayurkale22
Copy link
Member

It would be nice if you can update the Usage section in Readme (https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-exporter-zipkin/README.md) to highlight this new option, otherwise there is no easy way for new users to know about this option.

@obecny obecny requested a review from legendecas as a code owner June 17, 2020 11:56
@mayurkale22
Copy link
Member

/cc @astorm

@dyladan dyladan added the Merge:LGTM This PR is ready to be merged by a Maintainer (has enough valid approvals, successful build, etc.) label Jun 18, 2020
@mayurkale22 mayurkale22 merged commit 2afaffa into open-telemetry:master Jun 18, 2020
@obecny obecny deleted the zipkin-headers branch July 8, 2020 12:16
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Merge:LGTM This PR is ready to be merged by a Maintainer (has enough valid approvals, successful build, etc.) size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't Set Authorization Headers for Zipkin Exporter
5 participants