Metric exporter REUSABLE_DATA memory mode configuration options #6304

jack-berg · 2024-03-18T21:08:02Z

The work that has been done to introduce the REUSABLE_DATA memory mode dramatically reduces memory allocations for the metrics SDK. And because the SDK doesn't allow concurrent collections, it should be safe to use in built in exporters like OTLP and prometheus. (The only time it wouldn't be safe to use would be if an exporter held on to MetricData refs after completing the CompletableResultCode returned by export.)

This PR adds experimental API to programmatically set reusable data memory mode:

OtlpHttpMetricExporterBuilder builder = OtlpHttpMetricExporter.builder();
OtlpConfigUtil.setMemoryModeOnOtlpMetricExporterBuilder(builder, MemoryMode.REUSABLE_DATA);

And an experimental system property / env var for setting reusable memory mode:

OTEL_JAVA_EXPERIMENTAL_EXPORTER_MEMORY_MODE=REUSABLE_DATA

This will set memory mode for any exporter that supports setting reusable data memory mode. Currently, that include OtlpHttpMetricExporter, OtlpGrpcMetricExporter, and PrometheusHttpServer.

After we incubate this for a bit, we should make these configuration options part of the permanent public surface area, and probably enable REUSABLE_DATA mode by default for OTLP and prometheusmetrics, since its almost always the right thing to do.

cc @asafm

…terBuilder

codecov · 2024-03-18T21:16:03Z

Codecov Report

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

Project coverage is 91.09%. Comparing base (1dde255) to head (be86995).
Report is 11 commits behind head on main.

Files	Patch %	Lines
...lemetry/exporter/otlp/internal/OtlpConfigUtil.java	77.77%	3 Missing and 1 partial ⚠️
...lemetry/exporter/internal/ExporterBuilderUtil.java	77.77%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #6304      +/-   ##
============================================
- Coverage     91.11%   91.09%   -0.02%     
- Complexity     5760     5772      +12     
============================================
  Files           627      627              
  Lines         16799    16852      +53     
  Branches       1718     1720       +2     
============================================
+ Hits          15306    15351      +45     
- Misses          998     1006       +8     
  Partials        495      495

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

asafm

Thanks a lot for taking this. It's on my list, but I haven't gotten around to it yet.

asafm · 2024-03-26T18:32:08Z

I've added this in the feature implementation plan: #5105 (comment)

asafm · 2024-03-31T15:23:41Z

@jack-berg Do we want to add support also PrometheusHttpServer in another PR?

jack-berg · 2024-04-02T17:50:24Z

@jack-berg Do we want to add support also PrometheusHttpServer in another PR?

Yes I think its good to add support to prometheus. Wondering whether we'd want just a single property instead of multiple:

OTEL_JAVA_EXPERIMENTAL_EXPORTER_MEMORY_MODE=REUSABLE_DATA

This would apply to all exporters, current and future, which support memory mode configuration
As of this PR, only OTLP metric exporter supports this, but we know we want to add support for prometheus exporter, and probably for OTLP span and logger exporters in the future once we optimize the serialization piece.
A single property is simple and avoids the proliferation of several experimental env vars.
Multiple properties gives you more granularity, but what are the chances you actually want / need it? If you do want more granularity, can implement AutoConfigurationCustomizer.

asafm · 2024-04-03T12:51:02Z

@jack-berg Do we want to add support also PrometheusHttpServer in another PR?

Yes I think its good to add support to prometheus. Wondering whether we'd want just a single property instead of multiple:
OTEL_JAVA_EXPERIMENTAL_EXPORTER_MEMORY_MODE=REUSABLE_DATA
This would apply to all exporters, current and future, which support memory mode configuration

As of this PR, only OTLP metric exporter supports this, but we know we want to add support for prometheus exporter, and probably for OTLP span and logger exporters in the future once we optimize the serialization piece.

A single property is simple and avoids the proliferation of several experimental env vars.

Multiple properties gives you more granularity, but what are the chances you actually want / need it? If you do want more granularity, can implement AutoConfigurationCustomizer.

I favor your single property suggestion

…y-java into reuseable-memory-mode-env-var

jack-berg · 2024-04-03T17:52:44Z

@asafm I've pushed commits to:

Generalize the env var. Now OTEL_JAVA_EXPERIMENTAL_EXPORTER_MEMORY_MODE sets memory mode for all exporters which support it
Add setter for memory mode PrometheusHttpServerBuilder.

sdk-extensions/autoconfigure/README.md

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java

breedx-splk

Thanks for the explanations @jack-berg. Looks like a great start to configure a potentially amazing perf improvement.

asafm

Sorry for the late reply, last week I was busy for a lecture I gave in a conference.

asafm · 2024-04-07T12:28:59Z

...ometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java

+    this.prometheusRegistry = builder.prometheusRegistry;
+    this.otelScopeEnabled = builder.otelScopeEnabled;
+    this.allowedResourceAttributesFilter = builder.allowedResourceAttributesFilter;
+    this.executor = builder.executor;


Don't you want to copy the memory mode as well?

asafm · 2024-04-07T12:33:27Z

...ters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServer.java

@@ -85,6 +89,11 @@ public AggregationTemporality getAggregationTemporality(InstrumentType instrumen
    return prometheusMetricReader.getAggregationTemporality(instrumentType);
  }

+  @Override


The only difference I see in PrometheusHttpServer compared with PeriodMetricReader is that in Prometheus, multiple threads can call the metric producer collect(). I think if the mode is turned at REUSABLE_DATA, we should protect and make it only one request at a time.

There's a lock preventing concurrent reads from a particular meter here, but its not enough: A meter could be read from sequentially, but PrometheusHttpServer might still be serializing the MetricData from the first read when the second read begins and overwrites the data. So yes, a lock is needed when PrometheusHttpServer has reusable data enabled. This is interesting because it makes the tradeoff really clear: Do you want reduced memory allocation but no concurrent reads? Or higher memory allocation w/ concurrent reads?

Yes. I think we can place that in the javaDoc of the memory mode setter of the builder, once we fix the bug. Do we have place we write documentation of exporters in general ?

asafm · 2024-04-07T12:34:59Z

sdk-extensions/autoconfigure/README.md

+| otel.logs.exporter                          | OTEL_LOGS_EXPORTER                          | List of exporters to be used for logging, separated by commas. Default is `otlp`. `none` means no autoconfigured exporter.                                                                               |
+| otel.java.experimental.exporter.memory_mode | OTEL_JAVA_EXPERIMENTAL_EXPORTER_MEMORY_MODE | If `reusable_data`, enable reusable memory mode (on exporters which support it) to reduce allocations. Default is `immutable_data`. This option is experimental and subject to change or removal.**[1]** |
+
+**[1]**: NOTE: The exporters which adhere


Would you like me to add documentation to the website about memory mode?

I think there's an issue open somewhere to move all this autoconfigure docs to the website. For the moment, opentelemetry.io just links to this page for anything related to SDK environment variables. I think that's fine for now.

I guess I'm asking if we should elaborate more on the memory mode, or the configuration description suffices?

jack-berg added 2 commits March 18, 2024 15:41

Add setMemoryMode package private method to Otlp{Protocol}MetricExpor…

81aff57

…terBuilder

Add env var for enabling REUSABLE_DATA with otlp

a52fc64

jack-berg requested a review from a team March 18, 2024 21:08

asafm approved these changes Mar 26, 2024

View reviewed changes

asafm mentioned this pull request Mar 26, 2024

Support large number of metric data points by reducing memory allocation #5105

Closed

jack-berg added 2 commits April 3, 2024 12:39

Make env var generic, add support for PrometheusMetricReader

bf56ddd

Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…

e204ff3

…y-java into reuseable-memory-mode-env-var

jack-berg changed the title ~~OTLP metric exporter REUSABLE_DATA memory mode configuration options~~ Metric exporter REUSABLE_DATA memory mode configuration options Apr 3, 2024

Make PrometheusHttpServerBuilder#setMemoryMode public

df8a3fe

breedx-splk reviewed Apr 4, 2024

View reviewed changes

sdk-extensions/autoconfigure/README.md Outdated Show resolved Hide resolved

breedx-splk reviewed Apr 5, 2024

View reviewed changes

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java Show resolved Hide resolved

List supported exporters

be86995

breedx-splk approved these changes Apr 5, 2024

View reviewed changes

jack-berg merged commit 4d8f4f3 into open-telemetry:main Apr 5, 2024
18 checks passed

asafm reviewed Apr 7, 2024

View reviewed changes

jack-berg mentioned this pull request Apr 9, 2024

PrometheusHttpServer prevent concurrent reads when reusable memory mode #6371

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metric exporter REUSABLE_DATA memory mode configuration options #6304

Metric exporter REUSABLE_DATA memory mode configuration options #6304

jack-berg commented Mar 18, 2024 •

edited

Loading

codecov bot commented Mar 18, 2024 •

edited

Loading

asafm left a comment

asafm commented Mar 26, 2024

asafm commented Mar 31, 2024

jack-berg commented Apr 2, 2024

asafm commented Apr 3, 2024

jack-berg commented Apr 3, 2024

breedx-splk left a comment

asafm left a comment

asafm Apr 7, 2024

jack-berg Apr 7, 2024

asafm Apr 7, 2024

jack-berg Apr 7, 2024

asafm Apr 9, 2024

asafm Apr 7, 2024

jack-berg Apr 7, 2024

asafm Apr 9, 2024

Metric exporter REUSABLE_DATA memory mode configuration options #6304

Metric exporter REUSABLE_DATA memory mode configuration options #6304

Conversation

jack-berg commented Mar 18, 2024 • edited Loading

codecov bot commented Mar 18, 2024 • edited Loading

Codecov Report

asafm left a comment

Choose a reason for hiding this comment

asafm commented Mar 26, 2024

asafm commented Mar 31, 2024

jack-berg commented Apr 2, 2024

asafm commented Apr 3, 2024

jack-berg commented Apr 3, 2024

breedx-splk left a comment

Choose a reason for hiding this comment

asafm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jack-berg commented Mar 18, 2024 •

edited

Loading

codecov bot commented Mar 18, 2024 •

edited

Loading