Low allocation OTLP trace marshaler by laurit · Pull Request #6410 · open-telemetry/opentelemetry-java

laurit · 2024-04-26T06:51:07Z

Subset of #6328 that contains only trace marshaling.

Benchmark                                                                     (numSpans)  Mode  Cnt        Score     Error   Units
RequestMarshalBenchmarks.createCustomMarshal                                          16  avgt   10        6.150 ±   0.715   us/op
RequestMarshalBenchmarks.createCustomMarshal:gc.alloc.rate                            16  avgt   10     2835.838 ± 315.210  MB/sec
RequestMarshalBenchmarks.createCustomMarshal:gc.alloc.rate.norm                       16  avgt   10    18200.003 ±   0.001    B/op
RequestMarshalBenchmarks.createCustomMarshal:gc.count                                 16  avgt   10       78.000            counts
RequestMarshalBenchmarks.createCustomMarshal:gc.time                                  16  avgt   10       77.000                ms
RequestMarshalBenchmarks.createCustomMarshal                                         512  avgt   10      186.124 ±   8.696   us/op
RequestMarshalBenchmarks.createCustomMarshal:gc.alloc.rate                           512  avgt   10     2873.058 ± 126.706  MB/sec
RequestMarshalBenchmarks.createCustomMarshal:gc.alloc.rate.norm                      512  avgt   10   560383.738 ±  17.191    B/op
RequestMarshalBenchmarks.createCustomMarshal:gc.count                                512  avgt   10       71.000            counts
RequestMarshalBenchmarks.createCustomMarshal:gc.time                                 512  avgt   10       82.000                ms
RequestMarshalBenchmarks.createCustomMarshalLowAllocation                             16  avgt   10        3.822 ±   0.112   us/op
RequestMarshalBenchmarks.createCustomMarshalLowAllocation:gc.alloc.rate               16  avgt   10        0.001 ±   0.001  MB/sec
RequestMarshalBenchmarks.createCustomMarshalLowAllocation:gc.alloc.rate.norm          16  avgt   10        0.002 ±   0.001    B/op
RequestMarshalBenchmarks.createCustomMarshalLowAllocation:gc.count                    16  avgt   10          ≈ 0            counts
RequestMarshalBenchmarks.createCustomMarshalLowAllocation                            512  avgt   10      137.433 ±  10.486   us/op
RequestMarshalBenchmarks.createCustomMarshalLowAllocation:gc.alloc.rate              512  avgt   10        0.001 ±   0.001  MB/sec
RequestMarshalBenchmarks.createCustomMarshalLowAllocation:gc.alloc.rate.norm         512  avgt   10        0.074 ±   0.014    B/op
RequestMarshalBenchmarks.createCustomMarshalLowAllocation:gc.count                   512  avgt   10          ≈ 0            counts
RequestMarshalBenchmarks.marshalCustom                                                16  avgt   10       13.325 ±   0.362   us/op
RequestMarshalBenchmarks.marshalCustom:gc.alloc.rate                                  16  avgt   10     1314.425 ±  35.678  MB/sec
RequestMarshalBenchmarks.marshalCustom:gc.alloc.rate.norm                             16  avgt   10    18368.007 ±   0.001    B/op
RequestMarshalBenchmarks.marshalCustom:gc.count                                       16  avgt   10       44.000            counts
RequestMarshalBenchmarks.marshalCustom:gc.time                                        16  avgt   10       47.000                ms
RequestMarshalBenchmarks.marshalCustom                                               512  avgt   10      482.968 ±  11.295   us/op
RequestMarshalBenchmarks.marshalCustom:gc.alloc.rate                                 512  avgt   10     1106.838 ±  25.556  MB/sec
RequestMarshalBenchmarks.marshalCustom:gc.alloc.rate.norm                            512  avgt   10   560544.256 ±   0.008    B/op
RequestMarshalBenchmarks.marshalCustom:gc.count                                      512  avgt   10       37.000            counts
RequestMarshalBenchmarks.marshalCustom:gc.time                                       512  avgt   10       41.000                ms
RequestMarshalBenchmarks.marshalCustomLowAllocation                                   16  avgt   10       14.876 ±   1.089   us/op
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.alloc.rate                     16  avgt   10        5.652 ±   0.394  MB/sec
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.alloc.rate.norm                16  avgt   10       88.008 ±   0.001    B/op
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.count                          16  avgt   10        2.000            counts
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.time                           16  avgt   10        5.000                ms
RequestMarshalBenchmarks.marshalCustomLowAllocation                                  512  avgt   10      525.182 ±  53.841   us/op
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.alloc.rate                    512  avgt   10        0.161 ±   0.016  MB/sec
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.alloc.rate.norm               512  avgt   10       88.270 ±   0.027    B/op
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.count                         512  avgt   10          ≈ 0            counts
RequestMarshalBenchmarks.marshalJson                                                  16  avgt   10       50.843 ±   1.571   us/op
RequestMarshalBenchmarks.marshalJson:gc.alloc.rate                                    16  avgt   10      615.603 ±  18.741  MB/sec
RequestMarshalBenchmarks.marshalJson:gc.alloc.rate.norm                               16  avgt   10    32816.027 ±   0.003    B/op
RequestMarshalBenchmarks.marshalJson:gc.count                                         16  avgt   10       20.000            counts
RequestMarshalBenchmarks.marshalJson:gc.time                                          16  avgt   10       23.000                ms
RequestMarshalBenchmarks.marshalJson                                                 512  avgt   10     1564.466 ±  24.863   us/op
RequestMarshalBenchmarks.marshalJson:gc.alloc.rate                                   512  avgt   10      661.690 ±  10.372  MB/sec
RequestMarshalBenchmarks.marshalJson:gc.alloc.rate.norm                              512  avgt   10  1085588.150 ±  15.889    B/op
RequestMarshalBenchmarks.marshalJson:gc.count                                        512  avgt   10       23.000            counts
RequestMarshalBenchmarks.marshalJson:gc.time                                         512  avgt   10       27.000                ms
RequestMarshalBenchmarks.marshalJsonLowAllocation                                     16  avgt   10       51.248 ±   0.936   us/op
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.alloc.rate                       16  avgt   10      173.268 ±   3.179  MB/sec
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.alloc.rate.norm                  16  avgt   10     9312.028 ±   0.002    B/op
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.count                            16  avgt   10        5.000            counts
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.time                             16  avgt   10        5.000                ms
RequestMarshalBenchmarks.marshalJsonLowAllocation                                    512  avgt   10     1680.969 ± 147.464   us/op
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.alloc.rate                      512  avgt   10      203.218 ±  17.142  MB/sec
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.alloc.rate.norm                 512  avgt   10   357220.414 ±  16.837    B/op
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.count                           512  avgt   10        7.000            counts
RequestMarshalBenchmarks.marshalJsonLowAllocation:gc.time                            512  avgt   10        8.000                ms

codecov · 2024-04-26T07:04:04Z

Codecov Report

Attention: Patch coverage is 83.48348% with 110 lines in your changes are missing coverage. Please review.

Project coverage is 90.79%. Comparing base (51ff6bc) to head (b9aa0fb).
Report is 2 commits behind head on main.

Files	Patch %	Lines
...elemetry/exporter/internal/marshal/Serializer.java	59.75%	29 Missing and 4 partials ⚠️
...orter/internal/marshal/StatelessMarshalerUtil.java	79.74%	26 Missing and 6 partials ⚠️
...ry/exporter/internal/marshal/MarshalerContext.java	73.11%	22 Missing and 3 partials ⚠️
...rter/internal/otlp/KeyValueStatelessMarshaler.java	73.33%	6 Missing and 6 partials ⚠️
...internal/otlp/ArrayAnyValueStatelessMarshaler.java	77.77%	2 Missing and 2 partials ⚠️
...tlp/traces/LowAllocationTraceRequestMarshaler.java	76.47%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #6410      +/-   ##
============================================
- Coverage     91.12%   90.79%   -0.34%     
- Complexity     5856     6013     +157     
============================================
  Files           636      651      +15     
  Lines         17062    17709     +647     
  Branches       1733     1769      +36     
============================================
+ Hits          15548    16079     +531     
- Misses         1019     1113      +94     
- Partials        495      517      +22

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

breedx-splk · 2024-04-26T17:53:13Z

Rebase or merge #6411 ?

jack-berg

This is looking really good! Left several comment but they all should be easy to resolve.

...ters/common/src/main/java/io/opentelemetry/exporter/internal/marshal/StatelessMarshaler.java

.../src/main/java/io/opentelemetry/exporter/internal/otlp/DoubleAnyValueStatelessMarshaler.java

...n/src/main/java/io/opentelemetry/exporter/internal/otlp/ArrayAnyValueStatelessMarshaler.java

exporters/common/src/main/java/io/opentelemetry/exporter/internal/marshal/MarshalerUtil.java

...c/main/java/io/opentelemetry/exporter/internal/otlp/traces/SpanStatusStatelessMarshaler.java

...ain/java/io/opentelemetry/exporter/internal/otlp/traces/ResourceSpansStatelessMarshaler.java

exporters/common/src/main/java/io/opentelemetry/exporter/internal/marshal/Serializer.java

.../src/test/java/io/opentelemetry/exporter/internal/otlp/traces/TraceRequestMarshalerTest.java

.../java/io/opentelemetry/exporter/internal/otlp/traces/LowAllocationTraceRequestMarshaler.java

...lp/common/src/jmh/java/io/opentelemetry/exporter/internal/otlp/RequestMarshalBenchmarks.java

jack-berg · 2024-04-26T20:56:02Z

This is the part of the benchmark which gets me excited. marshalCustom is the current stateful marshaling. marshalCustomLowAllocation is what you've prototyped here.

Time to marshal shows a small dip from 13.325 to 14.876
Normalized gc allocation drops from 18,368 down to 88! Awesome!

RequestMarshalBenchmarks.marshalCustom                                                16  avgt   10       13.325 ±   0.362   us/op
RequestMarshalBenchmarks.marshalCustom:gc.alloc.rate                                  16  avgt   10     1314.425 ±  35.678  MB/sec
RequestMarshalBenchmarks.marshalCustom:gc.alloc.rate.norm                             16  avgt   10    18368.007 ±   0.001    B/op
RequestMarshalBenchmarks.marshalCustom:gc.count                                       16  avgt   10       44.000            counts
RequestMarshalBenchmarks.marshalCustom:gc.time                                        16  avgt   10       47.000                ms
RequestMarshalBenchmarks.marshalCustomLowAllocation                                   16  avgt   10       14.876 ±   1.089   us/op
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.alloc.rate                     16  avgt   10        5.652 ±   0.394  MB/sec
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.alloc.rate.norm                16  avgt   10       88.008 ±   0.001    B/op
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.count                          16  avgt   10        2.000            counts
RequestMarshalBenchmarks.marshalCustomLowAllocation:gc.time                           16  avgt   10        5.000                ms

jack-berg

Just a couple of loose ends at this point.

...rters/common/src/test/java/io/opentelemetry/exporter/internal/marshal/MarshalerUtilTest.java

.../common/src/main/java/io/opentelemetry/exporter/internal/marshal/StatelessMarshalerUtil.java

jack-berg

Great stuff! Very excited about this!

jack-berg · 2024-04-29T18:56:57Z

@open-telemetry/java-approvers does anyone intend on reviewing this? If not, I'll merge so we can begin the followup work.

OisCircle · 2025-07-01T16:08:30Z

public void serializeStringWithContext(
      ProtoFieldInfo field, @Nullable String string, MarshalerContext context) throws IOException {
    if (string == null || string.isEmpty()) {
      return;
    }
    if (context.marshalStringNoAllocation()) {
      writeString(field, string, context.getSize(), context);
    } else {
      byte[] valueUtf8 = context.getData(byte[].class);
      writeString(field, valueUtf8);
    }
  }

I've discovered that the logic in the if statement that detects empty strings or null values and returns is causing serialization issues. When I input a stringKey with an empty string value, the empty value gets ignored. This results in output like (example):

"key": "hello"

According to the OTEL protocol specification I've researched, the expected output should be:

"key": "hello",
"stringValue": ""

For detailed information, please refer to https://protobuf.dev/programming-guides/proto3/#default

Can we remove the logic below ?

|| string.isEmpty()

@jack-berg @laurit

jack-berg · 2025-07-01T18:29:03Z

Please open an issue @OisCircle so this comment doesn't get lost

laurit requested a review from a team April 26, 2024 06:51

Low allocation OTLP trace marshaler

3dc6585

laurit force-pushed the low-alloc-otlp-trace branch from cba329d to 3dc6585 Compare April 26, 2024 20:40

jack-berg reviewed Apr 26, 2024

View reviewed changes

...lp/common/src/jmh/java/io/opentelemetry/exporter/internal/otlp/RequestMarshalBenchmarks.java Outdated Show resolved Hide resolved

address review comments

bc20a8c

jack-berg reviewed Apr 29, 2024

View reviewed changes

...rters/common/src/test/java/io/opentelemetry/exporter/internal/marshal/MarshalerUtilTest.java Outdated Show resolved Hide resolved

.../common/src/main/java/io/opentelemetry/exporter/internal/marshal/StatelessMarshalerUtil.java Outdated Show resolved Hide resolved

laurit added 2 commits April 29, 2024 20:45

address review comments

2bb2ef0

add usage instructions

b9aa0fb

jack-berg approved these changes Apr 29, 2024

View reviewed changes

jack-berg merged commit 2e59f54 into open-telemetry:main May 1, 2024

laurit deleted the low-alloc-otlp-trace branch May 2, 2024 08:55

This was referenced May 2, 2024

Low allocation OTLP metrics marshaler #6422

Merged

Marshal span status description without allocation #6423

Merged

jack-berg mentioned this pull request May 24, 2024

OtlpJsonLogging{Signal}Exporters should have option to use low allocation OTLP marshalers #6474

Closed

Conversation

laurit commented Apr 26, 2024

Uh oh!

codecov bot commented Apr 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

breedx-splk commented Apr 26, 2024

Uh oh!

jack-berg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jack-berg commented Apr 26, 2024

Uh oh!

jack-berg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jack-berg left a comment

Choose a reason for hiding this comment

Uh oh!

jack-berg commented Apr 29, 2024

Uh oh!

OisCircle commented Jul 1, 2025

Uh oh!

jack-berg commented Jul 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Apr 26, 2024 •

edited

Loading