-
Notifications
You must be signed in to change notification settings - Fork 909
Bring metrics exemplar filter and reservoir out of internal
#7636
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
base: main
Are you sure you want to change the base?
Changes from all commits
e5f07c1
0f5e0a8
fca1416
ad832a2
6b0589a
3269221
b96c52c
44887f9
eb19261
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,46 @@ | ||
Comparing source compatibility of opentelemetry-sdk-metrics-1.55.0-SNAPSHOT.jar against opentelemetry-sdk-metrics-1.54.0.jar | ||
No changes. | ||
+++ NEW CLASS: PUBLIC(+) FINAL(+) io.opentelemetry.sdk.metrics.exemplar.AlwaysOffFilter (not serializable) | ||
+++ CLASS FILE FORMAT VERSION: 52.0 <- n.a. | ||
+++ NEW INTERFACE: io.opentelemetry.sdk.metrics.exemplar.ExemplarFilter | ||
+++ NEW SUPERCLASS: java.lang.Object | ||
+++ NEW METHOD: PUBLIC(+) boolean shouldSampleMeasurement(long, io.opentelemetry.api.common.Attributes, io.opentelemetry.context.Context) | ||
+++ NEW METHOD: PUBLIC(+) boolean shouldSampleMeasurement(double, io.opentelemetry.api.common.Attributes, io.opentelemetry.context.Context) | ||
+++ NEW CLASS: PUBLIC(+) FINAL(+) io.opentelemetry.sdk.metrics.exemplar.AlwaysOnFilter (not serializable) | ||
+++ CLASS FILE FORMAT VERSION: 52.0 <- n.a. | ||
+++ NEW INTERFACE: io.opentelemetry.sdk.metrics.exemplar.ExemplarFilter | ||
+++ NEW SUPERCLASS: java.lang.Object | ||
+++ NEW METHOD: PUBLIC(+) boolean shouldSampleMeasurement(long, io.opentelemetry.api.common.Attributes, io.opentelemetry.context.Context) | ||
+++ NEW METHOD: PUBLIC(+) boolean shouldSampleMeasurement(double, io.opentelemetry.api.common.Attributes, io.opentelemetry.context.Context) | ||
+++ NEW INTERFACE: PUBLIC(+) ABSTRACT(+) io.opentelemetry.sdk.metrics.exemplar.ExemplarFilter (not serializable) | ||
+++ CLASS FILE FORMAT VERSION: 52.0 <- n.a. | ||
+++ NEW SUPERCLASS: java.lang.Object | ||
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.metrics.exemplar.ExemplarFilter alwaysOff() | ||
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.metrics.exemplar.ExemplarFilter alwaysOn() | ||
+++ NEW METHOD: PUBLIC(+) ABSTRACT(+) boolean shouldSampleMeasurement(long, io.opentelemetry.api.common.Attributes, io.opentelemetry.context.Context) | ||
+++ NEW METHOD: PUBLIC(+) ABSTRACT(+) boolean shouldSampleMeasurement(double, io.opentelemetry.api.common.Attributes, io.opentelemetry.context.Context) | ||
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.metrics.exemplar.ExemplarFilter traceBased() | ||
+++ NEW INTERFACE: PUBLIC(+) ABSTRACT(+) io.opentelemetry.sdk.metrics.exemplar.ExemplarReservoir (not serializable) | ||
+++ CLASS FILE FORMAT VERSION: 52.0 <- n.a. | ||
GENERIC TEMPLATES: +++ T:io.opentelemetry.sdk.metrics.data.ExemplarData | ||
+++ NEW SUPERCLASS: java.lang.Object | ||
+++ NEW METHOD: PUBLIC(+) ABSTRACT(+) java.util.List<T> collectAndReset(io.opentelemetry.api.common.Attributes) | ||
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.metrics.exemplar.ExemplarReservoir<io.opentelemetry.sdk.metrics.data.DoubleExemplarData> doubleFixedSizeReservoir(io.opentelemetry.sdk.common.Clock, int, java.util.function.Supplier<java.util.Random>) | ||
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.metrics.exemplar.ExemplarReservoir<io.opentelemetry.sdk.metrics.data.DoubleExemplarData> doubleNoSamples() | ||
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.metrics.exemplar.ExemplarReservoir<T> filtered(io.opentelemetry.sdk.metrics.exemplar.ExemplarFilter, io.opentelemetry.sdk.metrics.exemplar.ExemplarReservoir<T>) | ||
GENERIC TEMPLATES: +++ T:io.opentelemetry.sdk.metrics.data.ExemplarData | ||
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.metrics.exemplar.ExemplarReservoir<io.opentelemetry.sdk.metrics.data.DoubleExemplarData> histogramBucketReservoir(io.opentelemetry.sdk.common.Clock, java.util.List<java.lang.Double>) | ||
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.metrics.exemplar.ExemplarReservoir<io.opentelemetry.sdk.metrics.data.LongExemplarData> longFixedSizeReservoir(io.opentelemetry.sdk.common.Clock, int, java.util.function.Supplier<java.util.Random>) | ||
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.metrics.exemplar.ExemplarReservoir<io.opentelemetry.sdk.metrics.data.LongExemplarData> longNoSamples() | ||
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.sdk.metrics.exemplar.ExemplarReservoir<T> longToDouble(io.opentelemetry.sdk.metrics.exemplar.ExemplarReservoir<T>) | ||
GENERIC TEMPLATES: +++ T:io.opentelemetry.sdk.metrics.data.ExemplarData | ||
+++ NEW METHOD: PUBLIC(+) ABSTRACT(+) void offerDoubleMeasurement(double, io.opentelemetry.api.common.Attributes, io.opentelemetry.context.Context) | ||
+++ NEW METHOD: PUBLIC(+) ABSTRACT(+) void offerLongMeasurement(long, io.opentelemetry.api.common.Attributes, io.opentelemetry.context.Context) | ||
+++ NEW CLASS: PUBLIC(+) FINAL(+) io.opentelemetry.sdk.metrics.exemplar.TraceBasedExemplarFilter (not serializable) | ||
+++ CLASS FILE FORMAT VERSION: 52.0 <- n.a. | ||
+++ NEW INTERFACE: io.opentelemetry.sdk.metrics.exemplar.ExemplarFilter | ||
+++ NEW SUPERCLASS: java.lang.Object | ||
+++ NEW METHOD: PUBLIC(+) boolean shouldSampleMeasurement(long, io.opentelemetry.api.common.Attributes, io.opentelemetry.context.Context) | ||
+++ NEW METHOD: PUBLIC(+) boolean shouldSampleMeasurement(double, io.opentelemetry.api.common.Attributes, io.opentelemetry.context.Context) | ||
*** MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder (not serializable) | ||
=== CLASS FILE FORMAT VERSION: 52.0 <- 52.0 | ||
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder setExemplarFilter(io.opentelemetry.sdk.metrics.exemplar.ExemplarFilter) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,10 +10,9 @@ | |
import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException; | ||
import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; | ||
import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; | ||
import io.opentelemetry.sdk.metrics.exemplar.ExemplarFilter; | ||
import io.opentelemetry.sdk.metrics.export.MetricExporter; | ||
import io.opentelemetry.sdk.metrics.export.MetricReader; | ||
import io.opentelemetry.sdk.metrics.internal.SdkMeterProviderUtil; | ||
import io.opentelemetry.sdk.metrics.internal.exemplar.ExemplarFilter; | ||
import io.opentelemetry.sdk.metrics.internal.state.MetricStorage; | ||
import java.io.Closeable; | ||
import java.util.Collections; | ||
|
@@ -39,20 +38,8 @@ static void configureMeterProvider( | |
List<Closeable> closeables) { | ||
|
||
// Configure default exemplar filters. | ||
String exemplarFilter = | ||
config.getString("otel.metrics.exemplar.filter", "trace_based").toLowerCase(Locale.ROOT); | ||
switch (exemplarFilter) { | ||
case "always_off": | ||
SdkMeterProviderUtil.setExemplarFilter(meterProviderBuilder, ExemplarFilter.alwaysOff()); | ||
break; | ||
case "always_on": | ||
SdkMeterProviderUtil.setExemplarFilter(meterProviderBuilder, ExemplarFilter.alwaysOn()); | ||
break; | ||
case "trace_based": | ||
default: | ||
SdkMeterProviderUtil.setExemplarFilter(meterProviderBuilder, ExemplarFilter.traceBased()); | ||
break; | ||
} | ||
ExemplarFilter filter = getExemplarFilter(config); | ||
meterProviderBuilder.setExemplarFilter(filter); | ||
|
||
Integer cardinalityLimit = config.getInt("otel.java.metrics.cardinality.limit"); | ||
if (cardinalityLimit == null) { | ||
|
@@ -76,6 +63,20 @@ static void configureMeterProvider( | |
reader, instrumentType -> resolvedCardinalityLimit)); | ||
} | ||
|
||
private static ExemplarFilter getExemplarFilter(ConfigProperties config) { | ||
String exemplarFilter = | ||
config.getString("otel.metrics.exemplar.filter", "trace_based").toLowerCase(Locale.ROOT); | ||
switch (exemplarFilter) { | ||
case "always_off": | ||
return ExemplarFilter.alwaysOff(); | ||
case "always_on": | ||
return ExemplarFilter.alwaysOn(); | ||
case "trace_based": | ||
default: | ||
return ExemplarFilter.traceBased(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be good to log unrecognized exemplar filter kind, unless there is some reason not to do that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not new code. We would do that in a follow-up PR if wanted. |
||
} | ||
} | ||
|
||
static List<MetricReader> configureMetricReaders( | ||
ConfigProperties config, | ||
SpiHelper spiHelper, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ | |
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.sdk.metrics.internal.exemplar; | ||
package io.opentelemetry.sdk.metrics.exemplar; | ||
|
||
import io.opentelemetry.api.common.Attributes; | ||
import io.opentelemetry.context.Context; | ||
|
@@ -20,9 +20,6 @@ | |
* An interface for an exemplar reservoir of samples. | ||
* | ||
* <p>This represents a reservoir for a specific "point" of metric data. | ||
* | ||
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change | ||
* at any time. | ||
*/ | ||
public interface ExemplarReservoir<T extends ExemplarData> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this guy need to be part of our public API? Do we want to allow people to create their own custom reservoir implementations? Just making sure we're not opening things up a bit too much before we know this is something that we want people to be able to customize. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cool. thanks for checking. 👍🏽 |
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some changes were made from the original exemplar design vs. what was ultimately marked stable. Notably, ExemplarFilter is no longer a extension plugin interface. See the difference in language between ExemplarFilter and ExemplarReservoir which is a extension plugin interface:
We need to adjust our API for ExemplarFilter accordingly. Its actually conceptually similar to our Aggregation: modeled as an interface with no methods, with the actual meaningful interface / methods in the internal AggregationFactory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so if I understand this correctly --
If I have that right, then it sounds like the course of action is for me to close this PR and copy the above comment into a new issue for us to fix, and make #7503 blocked/depend on that new issue? Yeah?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or you can leave this PR open / draft while separately pursue resolving the spec <-> opentelemetry-java deviation, and then rebase this PR.