Skip to content
This repository was archived by the owner on Dec 23, 2023. It is now read-only.

Add Tracestate into SpanContext. #1359

Merged
merged 3 commits into from
Aug 9, 2018

Conversation

bogdandrutu
Copy link
Contributor

One caveat in this PR: Tracestate name seems to not be consistent with the other class names. Maybe TraceState is better (just want to have others opinion).

@codecov-io
Copy link

codecov-io commented Aug 3, 2018

Codecov Report

Merging #1359 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1359      +/-   ##
============================================
+ Coverage     83.34%   83.36%   +0.02%     
- Complexity     1429     1433       +4     
============================================
  Files           223      223              
  Lines          6749     6758       +9     
  Branches        638      638              
============================================
+ Hits           5625     5634       +9     
  Misses          944      944              
  Partials        180      180
Impacted Files Coverage Δ Complexity Δ
...pencensus/implcore/trace/propagation/B3Format.java 100% <100%> (ø) 11 <1> (ø) ⬇️
...standard/util/AppEngineCloudTraceContextUtils.java 91.66% <100%> (+0.36%) 4 <0> (ø) ⬇️
...s/implcore/trace/propagation/BinaryFormatImpl.java 100% <100%> (ø) 12 <1> (+1) ⬆️
...src/main/java/io/opencensus/trace/SpanContext.java 92% <100%> (+1.52%) 17 <5> (+2) ⬆️
.../io/opencensus/implcore/trace/SpanBuilderImpl.java 96.29% <100%> (+0.14%) 28 <1> (+1) ⬆️
.../src/main/java/io/opencensus/trace/Tracestate.java 84.74% <100%> (-0.51%) 23 <1> (ø)
...opencensus/contrib/http/util/CloudTraceFormat.java 100% <100%> (ø) 14 <0> (ø) ⬇️

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 18aa279...11cd8ef. Read the comment docs.

@codefromthecrypt
Copy link
Contributor

FWIW I like the name as it matches the header. if we do otherwise tempts people to disregard that.

@codefromthecrypt
Copy link
Contributor

main concerns to consider is if we want this exposed as public api as it will be always available to user. for example it can be propagated package privately.. that is a choice especially as this isnt really a user api.

@bogdandrutu bogdandrutu added the kokoro:force-run Label to force a Kokoro build immediately. label Aug 3, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Label to force a Kokoro build immediately. label Aug 3, 2018
@songy23 songy23 added tracing kokoro:force-run Label to force a Kokoro build immediately. API labels Aug 3, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Label to force a Kokoro build immediately. label Aug 3, 2018
@sebright sebright requested review from dinooliva and removed request for sebright August 6, 2018 04:18
@bogdandrutu bogdandrutu requested a review from songy23 August 7, 2018 14:44
Copy link
Contributor

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

Please also add this change to CHANGELOG.md.

@bogdandrutu bogdandrutu merged commit 52f38e4 into census-instrumentation:master Aug 9, 2018
@bogdandrutu bogdandrutu deleted the tssp branch August 9, 2018 17:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants