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

Rework log fields #3837

Merged
merged 3 commits into from
Nov 10, 2021
Merged

Rework log fields #3837

merged 3 commits into from
Nov 10, 2021

Conversation

jack-berg
Copy link
Member

Solves several of the issues in #3804. Replaces #3811.

This PR includes a couple of changes to log fields, based on conversations in the 11/4 SIG:

  • setTraceId(..), setSpanId(..), and setFlags(..) are replaced with setContext(Context)
  • getTraceId(), getSpanId() and getFlags() are replaced with SpanContext getSpanContext()
  • setAttributes(..) now replaces attributes instead of merging them. This is less surprising.
  • setBody(Body) is removed. Only setBody(String) is currently supported. On the read side Body getBody() still returns a Body.
  • The nullability of fields has been been addressed. Generally, null is avoid if the response has an alternativel
    • SpanContext getSpanContext() returns SpanContext.getInvalid() if unset.
    • Severity getSeverity() returns Severity#UNDEFINED_SEVERITY_NUMBER if unset.
    • String getSeverityText() is nullable and returns null if unset.
    • String getName() is nullable and returns null if unset.
    • Body getBody() returns Body#emptyBody() if unset.
    • Attributes getAttributes() returns Attributes#empty() if unset.

@codecov
Copy link

codecov bot commented Nov 8, 2021

Codecov Report

Merging #3837 (a234bca) into main (fc68a88) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #3837      +/-   ##
============================================
+ Coverage     89.33%   89.38%   +0.04%     
- Complexity     4092     4098       +6     
============================================
  Files           488      489       +1     
  Lines         12630    12629       -1     
  Branches       1229     1231       +2     
============================================
+ Hits          11283    11288       +5     
+ Misses          926      923       -3     
+ Partials        421      418       -3     
Impacted Files Coverage Δ
...va/io/opentelemetry/sdk/logs/data/LogDataImpl.java 100.00% <ø> (ø)
...lemetry/exporter/logging/SystemOutLogExporter.java 94.11% <100.00%> (ø)
...etry/exporter/otlp/internal/logs/LogMarshaler.java 69.62% <100.00%> (+2.95%) ⬆️
.../java/io/opentelemetry/sdk/logs/SdkLogBuilder.java 100.00% <100.00%> (ø)
...main/java/io/opentelemetry/sdk/logs/data/Body.java 100.00% <100.00%> (ø)
...java/io/opentelemetry/sdk/logs/data/EmptyBody.java 100.00% <100.00%> (ø)
...io/opentelemetry/sdk/logs/data/LogDataBuilder.java 100.00% <100.00%> (ø)
...emetry/api/baggage/propagation/PercentEscaper.java 79.69% <0.00%> (-4.52%) ⬇️
...telemetry/sdk/trace/export/BatchSpanProcessor.java 90.07% <0.00%> (-0.71%) ⬇️
... and 7 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 fc68a88...a234bca. Read the comment docs.

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.

Looks much nicer thanks!

@jkwatson jkwatson merged commit cb057dd into open-telemetry:main Nov 10, 2021
@jack-berg jack-berg mentioned this pull request Nov 10, 2021
This was referenced Dec 19, 2021
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.

3 participants