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.

Sign up for GitHub

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

Metric exporter REUSABLE_DATA memory mode configuration options #6304

Merged
merged 6 commits into from
Apr 5, 2024
Merged

Metric exporter REUSABLE_DATA memory mode configuration options #6304

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
81aff57
Add setMemoryMode package private method to Otlp{Protocol}MetricExpor…
jack-berg Mar 18, 2024
a52fc64
Add env var for enabling REUSABLE_DATA with otlp
jack-berg Mar 18, 2024
bf56ddd
Make env var generic, add support for PrometheusMetricReader
jack-berg Apr 3, 2024
e204ff3
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
jack-berg Apr 3, 2024
df8a3fe
Make PrometheusHttpServerBuilder#setMemoryMode public
jack-berg Apr 3, 2024
be86995
List supported exporters
jack-berg Apr 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions exporters/common/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ otelJava.moduleName.set("io.opentelemetry.exporter.internal")
val versions: Map<String, String> by project
dependencies {
api(project(":api:all"))
api(project(":sdk-extensions:autoconfigure-spi"))

compileOnly(project(":sdk:common"))

Expand Down
21 changes: 21 additions & 0 deletions exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterBuilderUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,13 @@

package io.opentelemetry.exporter.internal;

import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
import io.opentelemetry.sdk.common.export.MemoryMode;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Locale;
import java.util.function.Consumer;

/**
* Utilities for exporter builders.
Expand All @@ -33,5 +38,21 @@
return uri;
}

/** Invoke the {@code memoryModeConsumer} with the configured {@link MemoryMode}. */
public static void configureExporterMemoryMode(
ConfigProperties config, Consumer<MemoryMode> memoryModeConsumer) {
String memoryModeStr = config.getString("otel.java.experimental.exporter.memory_mode");
if (memoryModeStr == null) {
return;
}
MemoryMode memoryMode;
try {
memoryMode = MemoryMode.valueOf(memoryModeStr.toUpperCase(Locale.ROOT));
} catch (IllegalArgumentException e) {
throw new ConfigurationException("Unrecognized memory mode: " + memoryModeStr, e);

Check warning on line 52 in exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterBuilderUtil.java

View check run for this annotation

Codecov / codecov/patch

exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterBuilderUtil.java#L51-L52 

Added lines #L51 - L52 were not covered by tests
}
memoryModeConsumer.accept(memoryMode);
}

private ExporterBuilderUtil() {}
}
17 changes: 0 additions & 17 deletions exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,23 +211,6 @@
}
}

/** Invoke the {@code memoryModeConsumer} with the configured {@link MemoryMode}. */
public static void configureOtlpMetricMemoryMode(
ConfigProperties config, Consumer<MemoryMode> memoryModeConsumer) {
String memoryModeStr =
config.getString("otel.java.experimental.exporter.otlp.metrics.memory_mode");
if (memoryModeStr == null) {
return;
}
MemoryMode memoryMode;
try {
memoryMode = MemoryMode.valueOf(memoryModeStr.toUpperCase(Locale.ROOT));
} catch (IllegalArgumentException e) {
throw new ConfigurationException("Unrecognized memory mode: " + memoryModeStr, e);
}
memoryModeConsumer.accept(memoryMode);
}

/**
* Calls {@code #setMemoryMode} on the {@code Otlp{Protocol}MetricExporterBuilder} with the {@code
* memoryMode}.
Expand Down Expand Up @@ -255,14 +238,14 @@
method.setAccessible(true);
method.invoke(builder, memoryMode);
} else {
throw new IllegalArgumentException(

Check warning on line 241 in exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L241 was not covered by tests
"Can only set memory mode on OtlpHttpMetricExporterBuilder and OtlpGrpcMetricExporterBuilder.");
}
} catch (NoSuchMethodException

Check warning on line 244 in exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L244 was not covered by tests
| InvocationTargetException
| IllegalAccessException
| ClassNotFoundException e) {
throw new IllegalStateException("Error calling setMemoryMode.", e);

Check warning on line 248 in exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L248 was not covered by tests
}
}

Expand Down
5 changes: 3 additions & 2 deletions ...all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpMetricExporterProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import static io.opentelemetry.exporter.otlp.internal.OtlpConfigUtil.PROTOCOL_GRPC;
import static io.opentelemetry.exporter.otlp.internal.OtlpConfigUtil.PROTOCOL_HTTP_PROTOBUF;

import io.opentelemetry.exporter.internal.ExporterBuilderUtil;
import io.opentelemetry.exporter.otlp.http.metrics.OtlpHttpMetricExporter;
import io.opentelemetry.exporter.otlp.http.metrics.OtlpHttpMetricExporterBuilder;
import io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter;
Expand Down Expand Up @@ -48,7 +49,7 @@ public MetricExporter createExporter(ConfigProperties config) {
config, builder::setAggregationTemporalitySelector);
OtlpConfigUtil.configureOtlpHistogramDefaultAggregation(
config, builder::setDefaultAggregationSelector);
OtlpConfigUtil.configureOtlpMetricMemoryMode(
ExporterBuilderUtil.configureExporterMemoryMode(
config,
memoryMode ->
OtlpConfigUtil.setMemoryModeOnOtlpMetricExporterBuilder(builder, memoryMode));
Expand All @@ -71,7 +72,7 @@ public MetricExporter createExporter(ConfigProperties config) {
config, builder::setAggregationTemporalitySelector);
OtlpConfigUtil.configureOtlpHistogramDefaultAggregation(
config, builder::setDefaultAggregationSelector);
OtlpConfigUtil.configureOtlpMetricMemoryMode(
ExporterBuilderUtil.configureExporterMemoryMode(
config,
memoryMode ->
OtlpConfigUtil.setMemoryModeOnOtlpMetricExporterBuilder(builder, memoryMode));
Expand Down
4 changes: 2 additions & 2 deletions ...src/test/java/io/opentelemetry/exporter/otlp/internal/OtlpMetricExporterProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ void createExporter_GrpcWithSignalConfiguration() throws CertificateEncodingExce
config.put("otel.exporter.otlp.metrics.compression", "gzip");
config.put("otel.exporter.otlp.timeout", "1s");
config.put("otel.exporter.otlp.metrics.timeout", "15s");
config.put("otel.java.experimental.exporter.otlp.metrics.memory_mode", "reusable_data");
config.put("otel.java.experimental.exporter.memory_mode", "reusable_data");

try (MetricExporter exporter =
provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
Expand Down Expand Up @@ -265,7 +265,7 @@ void createExporter_HttpWithSignalConfiguration() throws CertificateEncodingExce
config.put("otel.exporter.otlp.metrics.compression", "gzip");
config.put("otel.exporter.otlp.timeout", "1s");
config.put("otel.exporter.otlp.metrics.timeout", "15s");
config.put("otel.java.experimental.exporter.otlp.metrics.memory_mode", "reusable_data");
config.put("otel.java.experimental.exporter.memory_mode", "reusable_data");

try (MetricExporter exporter =
provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
Expand Down
1 change: 1 addition & 0 deletions exporters/prometheus/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ otelJava.moduleName.set("io.opentelemetry.exporter.prometheus")
dependencies {
api(project(":sdk:metrics"))

implementation(project(":exporters:common"))
implementation(project(":sdk-extensions:autoconfigure-spi"))
implementation("io.prometheus:prometheus-metrics-exporter-httpserver")

Expand Down
13 changes: 11 additions & 2 deletions ...s/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
package io.opentelemetry.exporter.prometheus;

import io.opentelemetry.sdk.common.CompletableResultCode;
import io.opentelemetry.sdk.common.export.MemoryMode;
import io.opentelemetry.sdk.metrics.InstrumentType;
import io.opentelemetry.sdk.metrics.data.AggregationTemporality;
import io.opentelemetry.sdk.metrics.export.CollectionRegistration;
Expand All @@ -32,11 +33,12 @@
*/
public final class PrometheusHttpServer implements MetricReader {

private final PrometheusHttpServerBuilder builder;
private final HTTPServer httpServer;
private final PrometheusMetricReader prometheusMetricReader;
private final PrometheusRegistry prometheusRegistry;
private final String host;
private final PrometheusHttpServerBuilder builder;
private final MemoryMode memoryMode;

/**
* Returns a new {@link PrometheusHttpServer} which can be registered to an {@link
Expand All @@ -59,11 +61,13 @@ public static PrometheusHttpServerBuilder builder() {
@Nullable ExecutorService executor,
PrometheusRegistry prometheusRegistry,
boolean otelScopeEnabled,
@Nullable Predicate<String> allowedResourceAttributesFilter) {
@Nullable Predicate<String> allowedResourceAttributesFilter,
MemoryMode memoryMode) {
this.builder = builder;
this.prometheusMetricReader =
new PrometheusMetricReader(otelScopeEnabled, allowedResourceAttributesFilter);
this.host = host;
this.memoryMode = memoryMode;
this.prometheusRegistry = prometheusRegistry;
prometheusRegistry.register(prometheusMetricReader);
try {
Expand All @@ -85,6 +89,11 @@ public AggregationTemporality getAggregationTemporality(InstrumentType instrumen
return prometheusMetricReader.getAggregationTemporality(instrumentType);
}

@Override
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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 ?

public MemoryMode getMemoryMode() {
return memoryMode;
}

@Override
public void register(CollectionRegistration registration) {
prometheusMetricReader.register(registration);
Expand Down
35 changes: 23 additions & 12 deletions ...theus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static io.opentelemetry.api.internal.Utils.checkArgument;
import static java.util.Objects.requireNonNull;

import io.opentelemetry.sdk.common.export.MemoryMode;
import io.prometheus.metrics.model.registry.PrometheusRegistry;
import java.util.concurrent.ExecutorService;
import java.util.function.Predicate;
Expand All @@ -18,13 +19,26 @@ public final class PrometheusHttpServerBuilder {

static final int DEFAULT_PORT = 9464;
private static final String DEFAULT_HOST = "0.0.0.0";
private static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.IMMUTABLE_DATA;

private String host = DEFAULT_HOST;
private int port = DEFAULT_PORT;
private PrometheusRegistry prometheusRegistry = new PrometheusRegistry();
private boolean otelScopeEnabled = true;
@Nullable private Predicate<String> allowedResourceAttributesFilter;
@Nullable private ExecutorService executor;
private MemoryMode memoryMode = DEFAULT_MEMORY_MODE;

PrometheusHttpServerBuilder() {}

PrometheusHttpServerBuilder(PrometheusHttpServerBuilder builder) {
this.host = builder.host;
this.port = builder.port;
this.prometheusRegistry = builder.prometheusRegistry;
this.otelScopeEnabled = builder.otelScopeEnabled;
this.allowedResourceAttributesFilter = builder.allowedResourceAttributesFilter;
this.executor = builder.executor;
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point

}

/** Sets the host to bind to. If unset, defaults to {@value #DEFAULT_HOST}. */
public PrometheusHttpServerBuilder setHost(String host) {
Expand Down Expand Up @@ -79,6 +93,13 @@ public PrometheusHttpServerBuilder setAllowedResourceAttributesFilter(
return this;
}

/** Set the {@link MemoryMode}. */
public PrometheusHttpServerBuilder setMemoryMode(MemoryMode memoryMode) {
requireNonNull(memoryMode, "memoryMode");
this.memoryMode = memoryMode;
return this;
}

/**
* Returns a new {@link PrometheusHttpServer} with the configuration of this builder which can be
* registered with a {@link io.opentelemetry.sdk.metrics.SdkMeterProvider}.
Expand All @@ -91,17 +112,7 @@ public PrometheusHttpServer build() {
executor,
prometheusRegistry,
otelScopeEnabled,
allowedResourceAttributesFilter);
}

PrometheusHttpServerBuilder() {}

PrometheusHttpServerBuilder(PrometheusHttpServerBuilder builder) {
this.host = builder.host;
this.port = builder.port;
this.prometheusRegistry = builder.prometheusRegistry;
this.otelScopeEnabled = builder.otelScopeEnabled;
this.allowedResourceAttributesFilter = builder.allowedResourceAttributesFilter;
this.executor = builder.executor;
allowedResourceAttributesFilter,
memoryMode);
}
}
3 changes: 3 additions & 0 deletions ...in/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package io.opentelemetry.exporter.prometheus.internal;

import io.opentelemetry.exporter.internal.ExporterBuilderUtil;
import io.opentelemetry.exporter.prometheus.PrometheusHttpServer;
import io.opentelemetry.exporter.prometheus.PrometheusHttpServerBuilder;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
Expand Down Expand Up @@ -32,6 +33,8 @@ public MetricReader createMetricReader(ConfigProperties config) {
prometheusBuilder.setHost(host);
}

ExporterBuilderUtil.configureExporterMemoryMode(config, prometheusBuilder::setMemoryMode);

return prometheusBuilder.build();
}

Expand Down
4 changes: 4 additions & 0 deletions ...ava/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import io.opentelemetry.exporter.prometheus.PrometheusHttpServer;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
import io.opentelemetry.sdk.common.export.MemoryMode;
import io.opentelemetry.sdk.metrics.export.MetricReader;
import io.prometheus.metrics.exporter.httpserver.HTTPServer;
import java.io.IOException;
Expand Down Expand Up @@ -57,6 +58,7 @@ void createMetricReader_Default() throws IOException {
assertThat(server.getAddress().getHostName()).isEqualTo("0:0:0:0:0:0:0:0");
assertThat(server.getAddress().getPort()).isEqualTo(9464);
});
assertThat(metricReader.getMemoryMode()).isEqualTo(MemoryMode.IMMUTABLE_DATA);
}
}

Expand All @@ -73,6 +75,7 @@ void createMetricReader_WithConfiguration() throws IOException {
Map<String, String> config = new HashMap<>();
config.put("otel.exporter.prometheus.host", "localhost");
config.put("otel.exporter.prometheus.port", String.valueOf(port));
config.put("otel.java.experimental.exporter.memory_mode", "reusable_data");

when(configProperties.getInt(any())).thenReturn(null);
when(configProperties.getString(any())).thenReturn(null);
Expand All @@ -87,6 +90,7 @@ void createMetricReader_WithConfiguration() throws IOException {
assertThat(server.getAddress().getHostName()).isEqualTo("localhost");
assertThat(server.getAddress().getPort()).isEqualTo(port);
});
assertThat(metricReader.getMemoryMode()).isEqualTo(MemoryMode.REUSABLE_DATA);
}
}
}
18 changes: 12 additions & 6 deletions sdk-extensions/autoconfigure/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,18 @@ The OpenTelemetry SDK can be disabled entirely. If disabled, `AutoConfiguredOpen

The following configuration properties are common to all exporters:

| System property | Environment variable | Purpose |
|-----------------------|-----------------------|----------------------------------------------------------------------------------------------------------------------------|
| otel.traces.exporter | OTEL_TRACES_EXPORTER | List of exporters to be used for tracing, separated by commas. Default is `otlp`. `none` means no autoconfigured exporter. |
| otel.metrics.exporter | OTEL_METRICS_EXPORTER | List of exporters to be used for metrics, separated by commas. Default is `otlp`. `none` means no autoconfigured exporter. |
| 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. |
| System property | Environment variable | Purpose |
|---------------------------------------------|---------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| otel.traces.exporter | OTEL_TRACES_EXPORTER | List of exporters to be used for tracing, separated by commas. Default is `otlp`. `none` means no autoconfigured exporter. |
| otel.metrics.exporter | OTEL_METRICS_EXPORTER | List of exporters to be used for metrics, separated by commas. Default is `otlp`. `none` means no autoconfigured exporter. |
| 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
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

to `otel.java.experimental.exporter.memory_mode=reusable_data`
are `OtlpGrpcMetricExporter`, `OtlpHttpMetricExporter`, and `PrometheusHttpServer`. Support for
additional exporters may be added in the future.


#### OTLP exporter (span, metric, and log exporters)

Expand Down Expand Up @@ -122,7 +129,6 @@ The [OpenTelemetry Protocol (OTLP)](https://github.com/open-telemetry/openteleme
| otel.exporter.otlp.metrics.temporality.preference | OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE | The preferred output aggregation temporality. Options include `DELTA`, `LOWMEMORY`, and `CUMULATIVE`. If `CUMULATIVE`, all instruments will have cumulative temporality. If `DELTA`, counter (sync and async) and histograms will be delta, up down counters (sync and async) will be cumulative. If `LOWMEMORY`, sync counter and histograms will be delta, async counter and up down counters (sync and async) will be cumulative. Default is `CUMULATIVE`. |
| otel.exporter.otlp.metrics.default.histogram.aggregation | OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION | The preferred default histogram aggregation. Options include `BASE2_EXPONENTIAL_BUCKET_HISTOGRAM` and `EXPLICIT_BUCKET_HISTOGRAM`. Default is `EXPLICIT_BUCKET_HISTOGRAM`. |
| otel.experimental.exporter.otlp.retry.enabled | OTEL_EXPERIMENTAL_EXPORTER_OTLP_RETRY_ENABLED | If `true`, enable [experimental retry support](#otlp-exporter-retry). Default is `false`. |
| otel.java.experimental.exporter.otlp.metrics.memory_mode | OTEL_JAVA_EXPERIMENTAL_EXPORTER_OTLP_METRICS_MEMORY_MODE | If `reusable_data`, enable reusable memory mode to reduce allocations. Default is `immutable_data`. This option is experimental and subject to change or removal. |

To configure the service name for the OTLP exporter, add the `service.name` key
to the OpenTelemetry Resource ([see below](#opentelemetry-resource)), e.g. `OTEL_RESOURCE_ATTRIBUTES=service.name=myservice`.
Expand Down
Loading