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

Some cleanup in span land. #2062

Merged
merged 6 commits into from Nov 12, 2020
Merged

Conversation

jkwatson
Copy link
Contributor

  • Clean up the SpanBuilder javadoc to match the current APIs
  • Scrub mentions of canonical status code
  • Deprecate the getCanonicalCode method on SpanBuilder and replace with getStatusCode

@codecov
Copy link

codecov bot commented Nov 11, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@9d17c66). Click here to learn what that means.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2062   +/-   ##
=========================================
  Coverage          ?   85.36%           
  Complexity        ?     2026           
=========================================
  Files             ?      231           
  Lines             ?     7849           
  Branches          ?      830           
=========================================
  Hits              ?     6700           
  Misses            ?      838           
  Partials          ?      311           
Impacted Files Coverage Δ Complexity Δ
...ava/io/opentelemetry/api/trace/PropagatedSpan.java 83.33% <ø> (ø) 18.00 <0.00> (?)
...src/main/java/io/opentelemetry/api/trace/Span.java 100.00% <ø> (ø) 17.00 <0.00> (?)
...n/java/io/opentelemetry/api/trace/SpanBuilder.java 50.00% <ø> (ø) 1.00 <0.00> (?)
...java/io/opentelemetry/exporter/jaeger/Adapter.java 97.19% <100.00%> (ø) 20.00 <0.00> (?)
...va/io/opentelemetry/exporter/otlp/SpanAdapter.java 96.11% <100.00%> (ø) 21.00 <0.00> (?)
...ntelemetry/exporter/zipkin/ZipkinSpanExporter.java 87.96% <100.00%> (ø) 30.00 <0.00> (?)
...emetry/sdk/extension/zpages/TracezSpanBuckets.java 100.00% <100.00%> (ø) 13.00 <0.00> (?)
...ntelemetry/sdk/trace/RecordEventsReadableSpan.java 84.28% <100.00%> (ø) 70.00 <4.00> (?)
.../opentelemetry/sdk/trace/data/ImmutableStatus.java 88.88% <100.00%> (ø) 6.00 <1.00> (?)
...java/io/opentelemetry/sdk/trace/data/SpanData.java 100.00% <100.00%> (ø) 0.00 <0.00> (?)
... and 231 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 9d17c66...3e10818. Read the comment docs.

*/
StatusCode getCanonicalCode();
@Deprecated
Copy link
Member

Choose a reason for hiding this comment

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

I think we currently still have enough other breaking changes that it is OK to completely remove this. It's just a renaming, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've started thinking that since we're moving to a 2-week release cycle, that it would be more friendly to end users to introduce deprecations that are then deleted in the following release. We discussed this at the SIG meeting this morning, and it was generally agreed to. This seemed like a easy way to ease into the practice. :)

Copy link
Contributor

@anuraaga anuraaga left a comment

Choose a reason for hiding this comment

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

Thanks for scrubbing!

jkwatson and others added 6 commits November 11, 2020 16:11
* Clean up the SpanBuilder javadoc to match the current APIs
* Scrub mentions of canonical status code
* Deprecate the getCanonicalCode method on SpanBuilder and replace with getStatusCode
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
@jkwatson jkwatson merged commit 1fe334f into open-telemetry:master Nov 12, 2020
@jkwatson jkwatson deleted the span_cleanups branch November 12, 2020 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants