Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Fix nested child span support for Zipkin #574

Merged

Conversation

hekike
Copy link
Contributor

@hekike hekike commented Jun 4, 2019

0.14.0 unfortunately, breaks nested child support in Zipkin.

  • fix support for nested child spans
  • test for published spans

Related to:
https://github.com/census-instrumentation/opencensus-node/pull/545/files#diff-427d5fa67f108e0a06e484bd47da9015R81

@hekike hekike changed the title fix(zipkin-exporter): fix nested child span support Fix nested child span support for Zipkin Jun 4, 2019
@codecov-io
Copy link

Codecov Report

Merging #574 into master will increase coverage by 0.24%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #574      +/-   ##
==========================================
+ Coverage   95.08%   95.33%   +0.24%     
==========================================
  Files         150      150              
  Lines        9877    10704     +827     
  Branches      725      756      +31     
==========================================
+ Hits         9392    10205     +813     
- Misses        485      499      +14
Impacted Files Coverage Δ
src/zpages-frontend/page-handlers/templates-dir.ts 80% <0%> (-20%) ⬇️
src/internal/cls.ts 91.66% <0%> (-8.34%) ⬇️
src/tags/propagation/text-format.ts 95.65% <0%> (-4.35%) ⬇️
src/zpages-frontend/latency-bucket-boundaries.ts 70.42% <0%> (-2.82%) ⬇️
src/tags/propagation/binary-serializer.ts 97.26% <0%> (-0.47%) ⬇️
src/internal/string-utils.ts 100% <0%> (ø) ⬆️
src/index.ts 100% <0%> (ø) ⬆️
test/test-tag-map.ts 100% <0%> (ø) ⬆️
test/test-no-record-span.ts 100% <0%> (ø) ⬆️
test/test-metric-producer-manager.ts 100% <0%> (ø) ⬆️
... and 34 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 405758f...b63c7b8. Read the comment docs.

Copy link
Member

@mayurkale22 mayurkale22 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 mayurkale22 merged commit 075bebd into census-instrumentation:master Jun 5, 2019
@mayurkale22
Copy link
Member

@hekike Let me know if you are waiting for this release?

@hekike hekike deleted the fix/zipkin-span-report branch June 11, 2019 17:15
@hekike
Copy link
Contributor Author

hekike commented Jun 11, 2019

@mayurkale22 thanks for asking, currently I just use 0.13.0, so it's not blocking.

@mayurkale22 mayurkale22 added this to the Release 0.0.15 milestone Jun 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants