Skip to content

Enable reusuable_data memory mode by default #6799

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

Merged
merged 4 commits into from
Oct 23, 2024
Merged

Enable reusuable_data memory mode by default #6799

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0d77447
Enable reusuable_data memory mode by default
jack-berg Oct 16, 2024
c02aedc
Fix typo
jack-berg Oct 16, 2024
6c7dc78
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
jack-berg Oct 21, 2024
a94bf75
Improve error message
jack-berg Oct 21, 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
12 changes: 11 additions & 1 deletion 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 @@ -21,6 +21,7 @@
import java.net.URISyntaxException;
import java.util.Locale;
import java.util.function.Consumer;
import java.util.logging.Logger;

/**
* Utilities for exporter builders.
Expand All @@ -30,6 +31,8 @@
*/
public final class ExporterBuilderUtil {

private static final Logger logger = Logger.getLogger(ExporterBuilderUtil.class.getName());

/** Validate OTLP endpoint. */
public static URI validateEndpoint(String endpoint) {
URI uri;
Expand All @@ -50,7 +53,14 @@
/** 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");
String memoryModeStr = config.getString("otel.java.exporter.memory_mode");
if (memoryModeStr == null) {
memoryModeStr = config.getString("otel.java.experimental.exporter.memory_mode");
if (memoryModeStr != null) {
logger.warning(

Check warning on line 60 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#L60 

Added line #L60 was not covered by tests
"otel.java.experimental.exporter.memory_mode was set but has been replaced with otel.java.exporter.memory_mode and will be removed in a future release");
}
}
if (memoryModeStr == null) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion .../main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
public final class OtlpHttpLogRecordExporterBuilder {

private static final String DEFAULT_ENDPOINT = "http://localhost:4318/v1/logs";
private static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.IMMUTABLE_DATA;
private static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.REUSABLE_DATA;

private final HttpExporterBuilder<Marshaler> delegate;
private MemoryMode memoryMode;
Expand Down
2 changes: 1 addition & 1 deletion .../main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public final class OtlpHttpMetricExporterBuilder {

private static final AggregationTemporalitySelector DEFAULT_AGGREGATION_TEMPORALITY_SELECTOR =
AggregationTemporalitySelector.alwaysCumulative();
private static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.IMMUTABLE_DATA;
private static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.REUSABLE_DATA;

private final HttpExporterBuilder<Marshaler> delegate;
private AggregationTemporalitySelector aggregationTemporalitySelector =
Expand Down
2 changes: 1 addition & 1 deletion .../src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
public final class OtlpHttpSpanExporterBuilder {

private static final String DEFAULT_ENDPOINT = "http://localhost:4318/v1/traces";
private static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.IMMUTABLE_DATA;
private static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.REUSABLE_DATA;

private final HttpExporterBuilder<Marshaler> delegate;
private MemoryMode memoryMode;
Expand Down
2 changes: 1 addition & 1 deletion ...l/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public final class OtlpGrpcLogRecordExporterBuilder {
private static final String DEFAULT_ENDPOINT_URL = "http://localhost:4317";
private static final URI DEFAULT_ENDPOINT = URI.create(DEFAULT_ENDPOINT_URL);
private static final long DEFAULT_TIMEOUT_SECS = 10;
private static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.IMMUTABLE_DATA;
private static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.REUSABLE_DATA;

// Visible for testing
final GrpcExporterBuilder<Marshaler> delegate;
Expand Down
2 changes: 1 addition & 1 deletion ...l/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public final class OtlpGrpcMetricExporterBuilder {
private static final long DEFAULT_TIMEOUT_SECS = 10;
private static final AggregationTemporalitySelector DEFAULT_AGGREGATION_TEMPORALITY_SELECTOR =
AggregationTemporalitySelector.alwaysCumulative();
private static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.IMMUTABLE_DATA;
private static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.REUSABLE_DATA;

// Visible for testing
final GrpcExporterBuilder<Marshaler> delegate;
Expand Down
2 changes: 1 addition & 1 deletion ...p/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public final class OtlpGrpcSpanExporterBuilder {
private static final String DEFAULT_ENDPOINT_URL = "http://localhost:4317";
private static final URI DEFAULT_ENDPOINT = URI.create(DEFAULT_ENDPOINT_URL);
private static final long DEFAULT_TIMEOUT_SECS = 10;
private static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.IMMUTABLE_DATA;
private static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.REUSABLE_DATA;

// Visible for testing
final GrpcExporterBuilder<Marshaler> delegate;
Expand Down
12 changes: 6 additions & 6 deletions .../test/java/io/opentelemetry/exporter/otlp/internal/OtlpLogRecordExporterProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ void createExporter_GrpcDefaults() {
verify(grpcBuilder, never()).setTrustedCertificates(any());
verify(grpcBuilder, never()).setClientTls(any(), any());
assertThat(grpcBuilder).extracting("delegate").extracting("retryPolicy").isNotNull();
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.IMMUTABLE_DATA);
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.REUSABLE_DATA);
}
Mockito.verifyNoInteractions(httpBuilder);
}
Expand Down Expand Up @@ -178,7 +178,7 @@ void createExporter_GrpcWithSignalConfiguration() throws CertificateEncodingExce
config.put("otel.exporter.otlp.logs.compression", "gzip");
config.put("otel.exporter.otlp.timeout", "1s");
config.put("otel.exporter.otlp.logs.timeout", "15s");
config.put("otel.java.experimental.exporter.memory_mode", "reusable_data");
config.put("otel.java.exporter.memory_mode", "immutable_data");

try (LogRecordExporter exporter =
provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
Expand All @@ -191,7 +191,7 @@ void createExporter_GrpcWithSignalConfiguration() throws CertificateEncodingExce
verify(grpcBuilder).setTrustedCertificates(serverTls.certificate().getEncoded());
verify(grpcBuilder)
.setClientTls(clientTls.privateKey().getEncoded(), clientTls.certificate().getEncoded());
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.REUSABLE_DATA);
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.IMMUTABLE_DATA);
}
Mockito.verifyNoInteractions(httpBuilder);
}
Expand All @@ -211,7 +211,7 @@ void createExporter_HttpDefaults() {
verify(httpBuilder, never()).setTrustedCertificates(any());
verify(httpBuilder, never()).setClientTls(any(), any());
assertThat(httpBuilder).extracting("delegate").extracting("retryPolicy").isNotNull();
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.IMMUTABLE_DATA);
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.REUSABLE_DATA);
}
Mockito.verifyNoInteractions(grpcBuilder);
}
Expand Down Expand Up @@ -264,7 +264,7 @@ void createExporter_HttpWithSignalConfiguration() throws CertificateEncodingExce
config.put("otel.exporter.otlp.logs.compression", "gzip");
config.put("otel.exporter.otlp.timeout", "1s");
config.put("otel.exporter.otlp.logs.timeout", "15s");
config.put("otel.java.experimental.exporter.memory_mode", "reusable_data");
config.put("otel.java.exporter.memory_mode", "immutable_data");

try (LogRecordExporter exporter =
provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
Expand All @@ -277,7 +277,7 @@ void createExporter_HttpWithSignalConfiguration() throws CertificateEncodingExce
verify(httpBuilder).setTrustedCertificates(serverTls.certificate().getEncoded());
verify(httpBuilder)
.setClientTls(clientTls.privateKey().getEncoded(), clientTls.certificate().getEncoded());
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.REUSABLE_DATA);
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.IMMUTABLE_DATA);
}
Mockito.verifyNoInteractions(grpcBuilder);
}
Expand Down
12 changes: 6 additions & 6 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 @@ -128,7 +128,7 @@ void createExporter_GrpcDefaults() {
verify(grpcBuilder, never()).setTrustedCertificates(any());
verify(grpcBuilder, never()).setClientTls(any(), any());
assertThat(grpcBuilder).extracting("delegate").extracting("retryPolicy").isNotNull();
assertThat(exporter.getMemoryMode()).isEqualTo(MemoryMode.IMMUTABLE_DATA);
assertThat(exporter.getMemoryMode()).isEqualTo(MemoryMode.REUSABLE_DATA);
}
Mockito.verifyNoInteractions(httpBuilder);
}
Expand Down 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.memory_mode", "reusable_data");
config.put("otel.java.exporter.memory_mode", "immutable_data");

try (MetricExporter exporter =
provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
Expand All @@ -191,7 +191,7 @@ void createExporter_GrpcWithSignalConfiguration() throws CertificateEncodingExce
verify(grpcBuilder).setTrustedCertificates(serverTls.certificate().getEncoded());
verify(grpcBuilder)
.setClientTls(clientTls.privateKey().getEncoded(), clientTls.certificate().getEncoded());
assertThat(exporter.getMemoryMode()).isEqualTo(MemoryMode.REUSABLE_DATA);
assertThat(exporter.getMemoryMode()).isEqualTo(MemoryMode.IMMUTABLE_DATA);
}
Mockito.verifyNoInteractions(httpBuilder);
}
Expand All @@ -212,7 +212,7 @@ void createExporter_HttpDefaults() {
verify(httpBuilder, never()).setTrustedCertificates(any());
verify(httpBuilder, never()).setClientTls(any(), any());
assertThat(httpBuilder).extracting("delegate").extracting("retryPolicy").isNotNull();
assertThat(exporter.getMemoryMode()).isEqualTo(MemoryMode.IMMUTABLE_DATA);
assertThat(exporter.getMemoryMode()).isEqualTo(MemoryMode.REUSABLE_DATA);
}
Mockito.verifyNoInteractions(grpcBuilder);
}
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.memory_mode", "reusable_data");
config.put("otel.java.exporter.memory_mode", "immutable_data");

try (MetricExporter exporter =
provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
Expand All @@ -278,7 +278,7 @@ void createExporter_HttpWithSignalConfiguration() throws CertificateEncodingExce
verify(httpBuilder).setTrustedCertificates(serverTls.certificate().getEncoded());
verify(httpBuilder)
.setClientTls(clientTls.privateKey().getEncoded(), clientTls.certificate().getEncoded());
assertThat(exporter.getMemoryMode()).isEqualTo(MemoryMode.REUSABLE_DATA);
assertThat(exporter.getMemoryMode()).isEqualTo(MemoryMode.IMMUTABLE_DATA);
}
Mockito.verifyNoInteractions(grpcBuilder);
}
Expand Down
14 changes: 7 additions & 7 deletions ...l/src/test/java/io/opentelemetry/exporter/otlp/internal/OtlpSpanExporterProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ void createExporter_GrpcDefaults() {
verify(grpcBuilder, never()).setTrustedCertificates(any());
verify(grpcBuilder, never()).setClientTls(any(), any());
assertThat(grpcBuilder).extracting("delegate").extracting("retryPolicy").isNotNull();
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.IMMUTABLE_DATA);
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.REUSABLE_DATA);
}
Mockito.verifyNoInteractions(httpBuilder);
}
Expand Down Expand Up @@ -179,7 +179,7 @@ void createExporter_GrpcWithSignalConfiguration() throws CertificateEncodingExce
config.put("otel.exporter.otlp.traces.compression", "gzip");
config.put("otel.exporter.otlp.timeout", "1s");
config.put("otel.exporter.otlp.traces.timeout", "15s");
config.put("otel.java.experimental.exporter.memory_mode", "reusable_data");
config.put("otel.java.exporter.memory_mode", "immutable_data");

try (SpanExporter exporter =
provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
Expand All @@ -192,7 +192,7 @@ void createExporter_GrpcWithSignalConfiguration() throws CertificateEncodingExce
verify(grpcBuilder).setTrustedCertificates(serverTls.certificate().getEncoded());
verify(grpcBuilder)
.setClientTls(clientTls.privateKey().getEncoded(), clientTls.certificate().getEncoded());
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.REUSABLE_DATA);
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.IMMUTABLE_DATA);
}
Mockito.verifyNoInteractions(httpBuilder);
}
Expand All @@ -212,7 +212,7 @@ void createExporter_HttpDefaults() {
verify(httpBuilder, never()).setTrustedCertificates(any());
verify(httpBuilder, never()).setClientTls(any(), any());
assertThat(httpBuilder).extracting("delegate").extracting("retryPolicy").isNotNull();
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.IMMUTABLE_DATA);
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.REUSABLE_DATA);
}
Mockito.verifyNoInteractions(grpcBuilder);
}
Expand Down Expand Up @@ -244,7 +244,7 @@ void createExporter_HttpWithGeneralConfiguration() throws CertificateEncodingExc
verify(httpBuilder)
.setClientTls(clientTls.privateKey().getEncoded(), clientTls.certificate().getEncoded());
assertThat(httpBuilder).extracting("delegate").extracting("retryPolicy").isNull();
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.IMMUTABLE_DATA);
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.REUSABLE_DATA);
}
Mockito.verifyNoInteractions(grpcBuilder);
}
Expand All @@ -268,7 +268,7 @@ void createExporter_HttpWithSignalConfiguration() throws CertificateEncodingExce
config.put("otel.exporter.otlp.traces.compression", "gzip");
config.put("otel.exporter.otlp.timeout", "1s");
config.put("otel.exporter.otlp.traces.timeout", "15s");
config.put("otel.java.experimental.exporter.memory_mode", "reusable_data");
config.put("otel.java.exporter.memory_mode", "immutable_data");

try (SpanExporter exporter =
provider.createExporter(DefaultConfigProperties.createFromMap(config))) {
Expand All @@ -281,7 +281,7 @@ void createExporter_HttpWithSignalConfiguration() throws CertificateEncodingExce
verify(httpBuilder).setTrustedCertificates(serverTls.certificate().getEncoded());
verify(httpBuilder)
.setClientTls(clientTls.privateKey().getEncoded(), clientTls.certificate().getEncoded());
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.REUSABLE_DATA);
assertThat(exporter).extracting("memoryMode").isEqualTo(MemoryMode.IMMUTABLE_DATA);
}
Mockito.verifyNoInteractions(grpcBuilder);
}
Expand Down
19 changes: 8 additions & 11 deletions ...rnal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/FakeTelemetryUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,19 @@ public class FakeTelemetryUtil {

private static final String TRACE_ID = "00000000000000000000000000abc123";
private static final String SPAN_ID = "0000000000def456";
private static final InstrumentationScopeInfo SCOPE_INFO =
InstrumentationScopeInfo.builder("testLib")
.setVersion("1.0")
.setSchemaUrl("http://url")
.build();

/** Generate a fake {@link MetricData}. */
public static MetricData generateFakeMetricData() {
long startNs = TimeUnit.MILLISECONDS.toNanos(System.currentTimeMillis());
long endNs = startNs + TimeUnit.MILLISECONDS.toNanos(900);
return ImmutableMetricData.createLongSum(
Resource.empty(),
InstrumentationScopeInfo.empty(),
SCOPE_INFO,
"name",
"description",
"1",
Expand Down Expand Up @@ -69,23 +74,15 @@ public static SpanData generateFakeSpanData() {
.setLinks(Collections.emptyList())
.setTotalRecordedLinks(0)
.setTotalRecordedEvents(0)
.setInstrumentationScopeInfo(
InstrumentationScopeInfo.builder("testLib")
.setVersion("1.0")
.setSchemaUrl("http://url")
.build())
.setInstrumentationScopeInfo(SCOPE_INFO)
.build();
}

/** Generate a fake {@link LogRecordData}. */
public static LogRecordData generateFakeLogRecordData() {
return TestLogRecordData.builder()
.setResource(Resource.getDefault())
.setInstrumentationScopeInfo(
InstrumentationScopeInfo.builder("testLib")
.setVersion("1.0")
.setSchemaUrl("http://url")
.build())
.setInstrumentationScopeInfo(SCOPE_INFO)
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 change seems random, but is actually oddly needed for a subtle reason. The OTLP exporter tests generate fake test data, serialize and send it to a mock server, where it is deserialized into generated proto classes and evaluated for equality. As you're probably aware, OTLP payloads consist of a series of enclosing envelopes, e.g. for logs ResourceLogs enclose ScopeLogs enclose LogRecords. When an OTLP exporter recieves a List<LogRecordData>, the records need to be grouped by resource and scope to match the OTLP payload structure. We do this using identity hash maps. Notably, the reuseable memory mode serialization iterates over identity the identity hash maps slightly differently than the immutable memory mode variant. This means that when a group of records is exported with scopes which are equal but not identical, the order of those ScopeLogs in the serialized payload is not the same for immutable memory mode and reuseable memory mode.

We can make the tests pass by having the fake log records use identical scopes, which sidesteps the difference in iteration order between immutable memory mode and reuseable memory mode.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the clarification....this is a clever workaround.

.setBody("log body")
.setAttributes(Attributes.builder().put("key", "value").build())
.setSeverity(Severity.INFO)
Expand Down
11 changes: 9 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 @@ -26,7 +26,8 @@
import java.io.UncheckedIOException;
import java.net.InetSocketAddress;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -82,7 +83,13 @@ public static PrometheusHttpServerBuilder builder() {
// sequentially.
if (memoryMode == MemoryMode.REUSABLE_DATA) {
executor =
Executors.newSingleThreadExecutor(new DaemonThreadFactory("prometheus-http-server"));
Copy link
Member Author

Choose a reason for hiding this comment

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

newSingleThreadExecutor returns a delegating wrapped ThreadPoolExecutor. One of our tests introspects on the resolved executor and ensures that when memory mode is reuseable, a single thread executor is used. The test is not able to make the required assertions based on the delegating wrapped version returned by Executors.newSingleThreadExecutor.

Copy link
Contributor

Choose a reason for hiding this comment

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

The way you described it makes it sound like a shortcoming in the test/assertions...which is a bummer that the implementation had to change to accommodate it. It seems fine, but I can't fully reason about the impact of having a non-finalizable executorservice. I'm overthinking it.

new ThreadPoolExecutor(
1,
1,
0L,
TimeUnit.MILLISECONDS,
new LinkedBlockingQueue<>(),
new DaemonThreadFactory("prometheus-http-server"));
}
try {
this.httpServer =
Expand Down
8 changes: 7 additions & 1 deletion ...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 @@ -24,7 +24,7 @@

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 static final MemoryMode DEFAULT_MEMORY_MODE = MemoryMode.REUSABLE_DATA;

private String host = DEFAULT_HOST;
private int port = DEFAULT_PORT;
Expand All @@ -46,6 +46,7 @@
this.otelScopeEnabled = builder.otelScopeEnabled;
this.allowedResourceAttributesFilter = builder.allowedResourceAttributesFilter;
this.executor = builder.executor;
this.memoryMode = builder.memoryMode;
this.defaultAggregationSelector = builder.defaultAggregationSelector;
}

Expand Down Expand Up @@ -150,6 +151,11 @@
* registered with a {@link io.opentelemetry.sdk.metrics.SdkMeterProvider}.
*/
public PrometheusHttpServer build() {
if (memoryMode == MemoryMode.REUSABLE_DATA && executor != null) {
throw new IllegalArgumentException(

Check warning on line 155 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerBuilder.java#L155 

Added line #L155 was not covered by tests
"MemoryMode is REUSEABLE_DATA cannot be used with custom executo, "
+ "since data may be corrupted if reading metrics concurrently");
}
return new PrometheusHttpServer(
new PrometheusHttpServerBuilder(this), // copy to prevent modification
host,
Expand Down
4 changes: 3 additions & 1 deletion ...ometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,8 @@ void customExecutor() throws IOException {
PrometheusHttpServer.builder()
.setHost("localhost")
.setPort(port)
// Memory mode must be IMMUTABLE_DATA to set custom executor
.setMemoryMode(MemoryMode.IMMUTABLE_DATA)
.setExecutor(scheduledExecutor)
.build()) {
assertThat(server)
Expand Down Expand Up @@ -520,7 +522,7 @@ void toBuilder() {
builder.setAllowedResourceAttributesFilter(resourceAttributesFilter);

ExecutorService executor = Executors.newSingleThreadExecutor();
builder.setExecutor(executor);
builder.setExecutor(executor).setMemoryMode(MemoryMode.IMMUTABLE_DATA);

PrometheusRegistry prometheusRegistry = new PrometheusRegistry();
builder.setPrometheusRegistry(prometheusRegistry);
Expand Down
Loading
Loading