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

LogData nullable spanId, traceId #3745

Merged
merged 2 commits into from
Oct 14, 2021

Conversation

jack-berg
Copy link
Member

Was working on a prototype Log4j2 appender, and noticed that protobuf marshaling fails if spanId / traceId are left to their default blank values.

I've updated LogData to reflect that spanId and traceId are optional, and updated the marshaler to work with nullable values.

Will likely to make other fields on LogData nullable as well.

}

@Test
void toProtoLogRecord_MinimalFields() {
Copy link
Member Author

Choose a reason for hiding this comment

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

This test fails without the updates to the marshaler.

@codecov
Copy link

codecov bot commented Oct 14, 2021

Codecov Report

Merging #3745 (c2259ed) into main (69d26c1) will increase coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #3745      +/-   ##
============================================
+ Coverage     89.18%   89.30%   +0.11%     
- Complexity     3870     3875       +5     
============================================
  Files           463      463              
  Lines         12125    12123       -2     
  Branches       1188     1188              
============================================
+ Hits          10814    10826      +12     
+ Misses          916      908       -8     
+ Partials        395      389       -6     
Impacted Files Coverage Δ
...etry/exporter/otlp/internal/logs/LogMarshaler.java 66.66% <ø> (ø)
.../io/opentelemetry/sdk/logs/LogSinkSdkProvider.java 100.00% <ø> (ø)
...java/io/opentelemetry/sdk/logs/data/LogRecord.java 100.00% <ø> (ø)
.../opentelemetry/sdk/logs/data/LogRecordBuilder.java 93.54% <ø> (-0.40%) ⬇️
...telemetry/sdk/trace/export/BatchSpanProcessor.java 90.78% <0.00%> (+0.70%) ⬆️
...elemetry/exporter/otlp/internal/MarshalerUtil.java 93.25% <0.00%> (+2.24%) ⬆️
...entelemetry/exporter/otlp/internal/Serializer.java 84.05% <0.00%> (+2.89%) ⬆️
...ntelemetry/sdk/extension/resources/OsResource.java 90.69% <0.00%> (+4.65%) ⬆️
...ava/io/opentelemetry/sdk/internal/RateLimiter.java 100.00% <0.00%> (+5.88%) ⬆️
...metry/sdk/extension/resources/ProcessResource.java 87.50% <0.00%> (+6.25%) ⬆️
... and 2 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 69d26c1...c2259ed. Read the comment docs.

Copy link
Contributor

@jkwatson jkwatson left a comment

Choose a reason for hiding this comment

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

👍🏽

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.

2 participants