Skip to content

Commit 0ef86c9

Browse files
authored
Fix runtime updates for metrics and views (#1856)
* enable runtime updates for views * update docs
1 parent b5ca4fa commit 0ef86c9

File tree

16 files changed

+197
-185
lines changed

16 files changed

+197
-185
lines changed

inspectit-ocelot-bootstrap/src/main/java/rocks/inspectit/ocelot/bootstrap/opentelemetry/IOpenTelemetryController.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,11 @@ default String getName() {
6868
boolean registerTraceExporterService(Object spanExporter, String serviceName);
6969

7070
/**
71-
* Registers a new {@link rocks.inspectit.ocelot.core.service.DynamicallyActivatableService metric reader} that is used to read recorded metrics.
71+
* Registers a metric reader, which is used to read recorded metrics.
7272
* <b><br/> IMPORTANT: This method should ONLY be used in tests of the {@code agent} package.</b>
7373
*
74-
* @param metricReader The {@link MetricReader}
75-
* @param serviceName The name of the metric reader service
74+
* @param metricReader The metric reader
75+
* @param serviceName The name of the service
7676
*
7777
* @return Whether the registration was successful
7878
*/

inspectit-ocelot-config/src/main/java/rocks/inspectit/ocelot/config/model/metrics/definition/views/ViewDefinitionSettings.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public class ViewDefinitionSettings {
3838
*/
3939
@NotNull
4040
@Builder.Default
41-
private AggregationType aggregation = AggregationType.LAST_VALUE;
41+
private AggregationType aggregation = AggregationType.LAST_VALUE; // TODO We should use the OTel DefaultAggregation for the instrument instead
4242

4343
/**
4444
* The maximum amount of unique combinations of attributes for this view.

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/exporter/LoggingMetricExporterService.java

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,30 +16,15 @@
1616
*/
1717
@Component
1818
@Slf4j
19-
public class LoggingMetricExporterService extends DynamicallyActivatableService {
19+
public class LoggingMetricExporterService extends DynamicallyActivatableService implements MetricReaderProvider {
2020

21-
/**
22-
* The {@link LoggingMetricExporter} for exporting metrics to the log
23-
*/
24-
private LoggingMetricExporter metricExporter;
25-
26-
/**
27-
* The {@link PeriodicMetricReader} for reading metrics to the log
28-
*/
29-
private MetricReader metricReader;
21+
/** The current exporter settings */
22+
private LoggingMetricsExporterSettings settings;
3023

3124
public LoggingMetricExporterService() {
3225
super("exporters.metrics.logging", "metrics.enabled");
3326
}
3427

35-
@Override
36-
protected void init() {
37-
super.init();
38-
39-
// create new metric exporter
40-
metricExporter = LoggingMetricExporter.create();
41-
}
42-
4328
@Override
4429
protected boolean checkEnabledForConfig(InspectitConfig configuration) {
4530
@Valid LoggingMetricsExporterSettings logging = configuration.getExporters().getMetrics().getLogging();
@@ -48,12 +33,10 @@ protected boolean checkEnabledForConfig(InspectitConfig configuration) {
4833

4934
@Override
5035
protected boolean doEnable(InspectitConfig configuration) {
51-
LoggingMetricsExporterSettings logging = configuration.getExporters().getMetrics().getLogging();
36+
settings = configuration.getExporters().getMetrics().getLogging();
5237
try {
53-
metricReader = PeriodicMetricReader.builder(metricExporter)
54-
.setInterval(logging.getExportInterval())
55-
.build();
56-
boolean success = openTelemetryController.registerMetricReader(metricReader, getName());
38+
boolean success = openTelemetryController.registerMetricReaderProvider(this, getName());
39+
5740
if (success) {
5841
log.info("Starting {}", getName());
5942
} else {
@@ -77,4 +60,12 @@ protected boolean doDisable() {
7760
return false;
7861
}
7962
}
63+
64+
@Override
65+
public MetricReader getNewMetricReader() {
66+
LoggingMetricExporter exporter = LoggingMetricExporter.create();
67+
return PeriodicMetricReader.builder(exporter)
68+
.setInterval(settings.getExportInterval())
69+
.build();
70+
}
8071
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package rocks.inspectit.ocelot.core.exporter;
2+
3+
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
4+
import io.opentelemetry.sdk.metrics.export.MetricReader;
5+
6+
public interface MetricReaderProvider {
7+
8+
/**
9+
* Gets a new {@link MetricReader} for this service.
10+
* It is important that this method returns a <strong>new</strong> {@link MetricReader},
11+
* as when the previously used {@link MetricReader} is shut down during {@link SdkMeterProvider#shutdown()},
12+
* it cannot be re-enabled.
13+
*
14+
* @return A new {@link MetricReader}
15+
*/
16+
MetricReader getNewMetricReader();
17+
}

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/exporter/OtlpMetricsExporterService.java

Lines changed: 60 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import io.opentelemetry.exporter.otlp.http.metrics.OtlpHttpMetricExporterBuilder;
66
import io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter;
77
import io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporterBuilder;
8+
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
89
import io.opentelemetry.sdk.metrics.data.AggregationTemporality;
910
import io.opentelemetry.sdk.metrics.export.*;
1011
import lombok.extern.slf4j.Slf4j;
@@ -27,20 +28,12 @@
2728
*/
2829
@Component
2930
@Slf4j
30-
public class OtlpMetricsExporterService extends DynamicallyActivatableService {
31+
public class OtlpMetricsExporterService extends DynamicallyActivatableService implements MetricReaderProvider {
3132

3233
private final List<TransportProtocol> SUPPORTED_PROTOCOLS = Arrays.asList(TransportProtocol.GRPC, TransportProtocol.HTTP_PROTOBUF);
3334

34-
/**
35-
* The {@link MetricExporter} for exporting metrics via OTLP
36-
*/
37-
@VisibleForTesting
38-
MetricExporter metricExporter;
39-
40-
/**
41-
* The {@link PeriodicMetricReader} for reading metrics
42-
*/
43-
private MetricReader metricReader;
35+
/** The current exporter settings */
36+
private OtlpMetricsExporterSettings settings;
4437

4538
public OtlpMetricsExporterService() {
4639
super("metrics.enabled", "exporters.metrics.otlp");
@@ -72,46 +65,12 @@ protected boolean checkEnabledForConfig(InspectitConfig configuration) {
7265
@Override
7366
protected boolean doEnable(InspectitConfig configuration) {
7467
try {
75-
OtlpMetricsExporterSettings otlp = configuration.getExporters().getMetrics().getOtlp();
76-
AggregationTemporalitySelector aggregationTemporalitySelector = otlp.getPreferredTemporality() == AggregationTemporality.DELTA ? AggregationTemporalitySelector.deltaPreferred() : AggregationTemporalitySelector.alwaysCumulative();
77-
78-
switch (otlp.getProtocol()) {
79-
case GRPC: {
80-
OtlpGrpcMetricExporterBuilder metricExporterBuilder = OtlpGrpcMetricExporter.builder()
81-
.setAggregationTemporalitySelector(aggregationTemporalitySelector)
82-
.setEndpoint(otlp.getEndpoint())
83-
.setCompression(otlp.getCompression().toString())
84-
.setTimeout(otlp.getTimeout());
85-
if (otlp.getHeaders() != null) {
86-
for (Map.Entry<String, String> headerEntry : otlp.getHeaders().entrySet()) {
87-
metricExporterBuilder.addHeader(headerEntry.getKey(), headerEntry.getValue());
88-
}
89-
}
90-
metricExporter = metricExporterBuilder.build();
91-
break;
92-
}
93-
case HTTP_PROTOBUF: {
94-
OtlpHttpMetricExporterBuilder metricExporterBuilder = OtlpHttpMetricExporter.builder()
95-
.setAggregationTemporalitySelector(aggregationTemporalitySelector)
96-
.setEndpoint(otlp.getEndpoint())
97-
.setCompression(otlp.getCompression().toString())
98-
.setTimeout(otlp.getTimeout());
99-
if (otlp.getHeaders() != null) {
100-
for (Map.Entry<String, String> headerEntry : otlp.getHeaders().entrySet()) {
101-
metricExporterBuilder.addHeader(headerEntry.getKey(), headerEntry.getValue());
102-
}
103-
}
104-
metricExporter = metricExporterBuilder.build();
105-
break;
106-
}
107-
}
108-
metricReader = PeriodicMetricReader.builder(metricExporter)
109-
.setInterval(otlp.getExportInterval())
110-
.build();
68+
settings = configuration.getExporters().getMetrics().getOtlp();
69+
70+
boolean success = openTelemetryController.registerMetricReaderProvider(this, getName());
11171

112-
boolean success = openTelemetryController.registerMetricReader(metricReader, getName());
11372
if (success) {
114-
log.info("Starting {} with protocol {} on endpoint {}", getName(), otlp.getProtocol(), otlp.getEndpoint());
73+
log.info("Starting {} with protocol {} on endpoint {}", getName(), settings.getProtocol(), settings.getEndpoint());
11574
} else {
11675
log.error("Failed to register {} at the OpenTelemetry controller!", getName());
11776
}
@@ -133,4 +92,56 @@ protected boolean doDisable() {
13392
return false;
13493
}
13594
}
95+
96+
@Override
97+
public MetricReader getNewMetricReader() {
98+
MetricExporter exporter = getNewMetricExporter();
99+
return PeriodicMetricReader.builder(exporter)
100+
.setInterval(settings.getExportInterval())
101+
.build();
102+
}
103+
104+
/**
105+
* We also create a new metric exporter, since the previous one will be unusable,
106+
* if we call {@code shutdown} on the previous {@link SdkMeterProvider}.
107+
*
108+
* @return the newly created metric exporter
109+
*/
110+
@VisibleForTesting
111+
MetricExporter getNewMetricExporter() {
112+
AggregationTemporalitySelector aggregationTemporalitySelector = settings.getPreferredTemporality() == AggregationTemporality.DELTA ?
113+
AggregationTemporalitySelector.deltaPreferred() :
114+
AggregationTemporalitySelector.alwaysCumulative();
115+
116+
switch (settings.getProtocol()) {
117+
case GRPC: {
118+
OtlpGrpcMetricExporterBuilder metricExporterBuilder = OtlpGrpcMetricExporter.builder()
119+
.setAggregationTemporalitySelector(aggregationTemporalitySelector)
120+
.setEndpoint(settings.getEndpoint())
121+
.setCompression(settings.getCompression().toString())
122+
.setTimeout(settings.getTimeout());
123+
if (settings.getHeaders() != null) {
124+
for (Map.Entry<String, String> headerEntry : settings.getHeaders().entrySet()) {
125+
metricExporterBuilder.addHeader(headerEntry.getKey(), headerEntry.getValue());
126+
}
127+
}
128+
return metricExporterBuilder.build();
129+
}
130+
case HTTP_PROTOBUF: {
131+
OtlpHttpMetricExporterBuilder metricExporterBuilder = OtlpHttpMetricExporter.builder()
132+
.setAggregationTemporalitySelector(aggregationTemporalitySelector)
133+
.setEndpoint(settings.getEndpoint())
134+
.setCompression(settings.getCompression().toString())
135+
.setTimeout(settings.getTimeout());
136+
if (settings.getHeaders() != null) {
137+
for (Map.Entry<String, String> headerEntry : settings.getHeaders().entrySet()) {
138+
metricExporterBuilder.addHeader(headerEntry.getKey(), headerEntry.getValue());
139+
}
140+
}
141+
return metricExporterBuilder.build();
142+
}
143+
default:
144+
throw new IllegalArgumentException("Unknown OpenTelemetry protocol: " + settings.getProtocol());
145+
}
146+
}
136147
}

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/exporter/PrometheusExporterService.java

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,10 @@
1414
*/
1515
@Component
1616
@Slf4j
17-
public class PrometheusExporterService extends DynamicallyActivatableService {
17+
public class PrometheusExporterService extends DynamicallyActivatableService implements MetricReaderProvider {
1818

19-
/**
20-
* The {@link PrometheusHttpServer} exposing recorded metrics
21-
*/
22-
private MetricReader metricReader;
19+
/** The current exporter settings */
20+
private PrometheusExporterSettings settings;
2321

2422
public PrometheusExporterService() {
2523
super("exporters.metrics.prometheus", "metrics.enabled");
@@ -36,19 +34,13 @@ protected boolean checkEnabledForConfig(InspectitConfig conf) {
3634

3735
@Override
3836
protected boolean doEnable(InspectitConfig configuration) {
39-
PrometheusExporterSettings config = configuration.getExporters().getMetrics().getPrometheus();
37+
settings = configuration.getExporters().getMetrics().getPrometheus();
4038

4139
try {
42-
String host = config.getHost();
43-
int port = config.getPort();
44-
metricReader = PrometheusHttpServer.builder()
45-
.setHost(host)
46-
.setPort(port)
47-
.build();
48-
boolean success = openTelemetryController.registerMetricReader(metricReader, getName());
40+
boolean success = openTelemetryController.registerMetricReaderProvider(this, getName());
4941

5042
if (success) {
51-
log.info("Starting Prometheus Exporter on {}:{}", host, port);
43+
log.info("Starting Prometheus Exporter on {}:{}", settings.getHost(), settings.getPort());
5244
} else {
5345
log.error("Failed to register {} at the OpenTelemetry controller!", getName());
5446
}
@@ -65,4 +57,12 @@ protected boolean doDisable() {
6557
openTelemetryController.unregisterMetricExporterService(getName());
6658
return true;
6759
}
60+
61+
@Override
62+
public MetricReader getNewMetricReader() {
63+
return PrometheusHttpServer.builder()
64+
.setHost(settings.getHost())
65+
.setPort(settings.getPort())
66+
.build();
67+
}
6868
}

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/InstrumentManager.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import io.opentelemetry.api.baggage.Baggage;
44
import io.opentelemetry.api.common.Attributes;
55
import io.opentelemetry.api.metrics.*;
6+
import io.opentelemetry.sdk.metrics.SdkMeterProvider;
67
import lombok.extern.slf4j.Slf4j;
78
import lombok.val;
89
import org.springframework.beans.factory.annotation.Autowired;
@@ -57,42 +58,42 @@ public class InstrumentManager {
5758
/**
5859
* Updates the instruments defined via {@link MetricsSettings#getDefinitions()}.
5960
* We should only update the instruments after the OpenTelemetry SDK has been configured.
60-
* Otherwise, we will create NOOP-instruments.
6161
*/
6262
@EventListener
6363
public void updateInstruments(OpenTelemetryConfiguredEvent event) {
6464
MetricsSettings metricsSettings = env.getCurrentConfig().getMetrics();
6565

6666
if (event.isSuccess() && metricsSettings.isEnabled()) {
67-
Set<String> instrumentsToRemove = processInstrumentUpdates(metricsSettings.getDefinitions());
67+
// TODO Maybe we can reuse the existing instruments for the new sdk meter via reflection?
68+
Set<String> instrumentsToRemove = processInstrumentUpdates(metricsSettings.getDefinitions(), event.isUpdateMetrics());
6869
instrumentsToRemove.forEach(this::removeInstrument);
6970
log.info("Successfully updated OpenTelemetry instruments");
7071
}
7172
}
7273

7374
/**
7475
* Processes the provided metric definitions to update {@link #cachedInstruments} and collects a set of
75-
* instrument names, which are no longer required.
76+
* instrument names, which are no longer required. <br>
7677
*
7778
* @param newDefinitions the new metric definitions
79+
* @param forceUpdate if we have to update all existing instruments
7880
*
7981
* @return the set of instrument names, which are no longer required
8082
*/
81-
public Set<String> processInstrumentUpdates(Map<String, MetricDefinitionSettings> newDefinitions) {
83+
public Set<String> processInstrumentUpdates(Map<String, MetricDefinitionSettings> newDefinitions, boolean forceUpdate) {
8284
Set<String> instrumentsToRemove = new HashSet<>(cachedInstruments.keySet());
8385

8486
newDefinitions.forEach((name, def) -> {
8587
val defWithDefaults = def.getCopyWithDefaultsPopulated(name);
8688
val currentDef = currentMetricDefinitions.get(name);
8789

8890
boolean instrumentRequired = cachedInstruments.containsKey(name);
89-
if (!defWithDefaults.equals(currentDef)) {
91+
if (forceUpdate || !defWithDefaults.equals(currentDef)) {
9092
instrumentRequired = updateInstrument(name, defWithDefaults);
9193
}
9294
if (instrumentRequired) {
9395
instrumentsToRemove.remove(name);
9496
}
95-
9697
});
9798

9899
return instrumentsToRemove;

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/jmx/JmxMetricsRecorder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ private void registerMetric(String metricName, String attrDescription, Map<Strin
146146
.getCopyWithDefaultsPopulated(metricName);
147147

148148
Map<String, MetricDefinitionSettings> metric = Collections.singletonMap(metricName, definitionSettingsWithLastValueView);
149-
instrumentManager.processInstrumentUpdates(metric);
149+
instrumentManager.processInstrumentUpdates(metric, false);
150150
}
151151

152152
/**

0 commit comments

Comments
 (0)