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(@opentelemetry/exporter-jaeger): fixed issue #1757 #1758

Merged
merged 2 commits into from
Dec 17, 2020

Conversation

debagger
Copy link
Contributor

Which problem is this PR solving?

Jaeger exporter set error tag when span status is Unset as discribed in issue #1757

Short description of the changes

Changed error flag setting logic
Added test for this

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 17, 2020

CLA Signed

The committers are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Dec 17, 2020

Codecov Report

Merging #1758 (8c8d607) into master (c5cbc49) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1758   +/-   ##
=======================================
  Coverage   92.07%   92.07%           
=======================================
  Files         166      166           
  Lines        5593     5593           
  Branches     1199     1199           
=======================================
  Hits         5150     5150           
  Misses        443      443           
Impacted Files Coverage Δ
...ges/opentelemetry-exporter-jaeger/src/transform.ts 100.00% <100.00%> (ø)

Copy link
Member

@obecny obecny left a comment

Choose a reason for hiding this comment

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

lgtm

@obecny obecny added the bug Something isn't working label Dec 17, 2020
@dyladan dyladan merged commit 14818da into open-telemetry:master Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants