From 5641021a3a74b4b46cbea9334506ebb82449cfef Mon Sep 17 00:00:00 2001 From: Matt Richardson Date: Tue, 3 Dec 2024 16:27:23 +1100 Subject: [PATCH] Dont use globals for the tc metrics settings (#343) --- .../endpoints/IOTELEndpointHandler.java | 4 +- .../custom/CustomOTELEndpointHandler.java | 6 ++- .../HoneycombOTELEndpointHandler.java | 35 +++++++++-------- .../zipkin/ZipkinOTELEndpointHandler.java | 6 ++- .../HelperPerBuildOTELHelperFactory.java | 8 ++-- .../server/helpers/OTELHelperImpl.java | 12 +++++- .../server/helpers/OTELMetrics.java | 38 ++++--------------- .../opentelemetry/server/OTELHelperTest.java | 2 +- .../server/TeamCityBuildListenerTest.java | 2 +- 9 files changed, 57 insertions(+), 56 deletions(-) diff --git a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/IOTELEndpointHandler.java b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/IOTELEndpointHandler.java index 823ee76..13ea219 100644 --- a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/IOTELEndpointHandler.java +++ b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/IOTELEndpointHandler.java @@ -1,9 +1,11 @@ package com.octopus.teamcity.opentelemetry.server.endpoints; import com.octopus.teamcity.opentelemetry.server.SetProjectConfigurationSettingsRequest; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.trace.SpanProcessor; import jetbrains.buildServer.serverSide.BuildPromotion; import jetbrains.buildServer.serverSide.SBuild; +import org.apache.commons.lang3.tuple.Pair; import org.springframework.web.servlet.ModelAndView; import javax.servlet.http.HttpServletRequest; @@ -12,7 +14,7 @@ public interface IOTELEndpointHandler { ModelAndView getBuildOverviewModelAndView(SBuild build, Map params, String traceId); - SpanProcessor buildSpanProcessor(BuildPromotion buildPromotion, String endpoint, Map params); + Pair buildSpanProcessorAndMeterProvider(BuildPromotion buildPromotion, String endpoint, Map params); SetProjectConfigurationSettingsRequest getSetProjectConfigurationSettingsRequest(HttpServletRequest request); diff --git a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/custom/CustomOTELEndpointHandler.java b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/custom/CustomOTELEndpointHandler.java index 88e6f3c..65ee6b7 100644 --- a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/custom/CustomOTELEndpointHandler.java +++ b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/custom/CustomOTELEndpointHandler.java @@ -4,6 +4,7 @@ import com.octopus.teamcity.opentelemetry.server.endpoints.IOTELEndpointHandler; import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporter; import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporterBuilder; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.trace.SpanProcessor; import io.opentelemetry.sdk.trace.export.BatchSpanProcessor; import io.opentelemetry.sdk.trace.export.SpanExporter; @@ -11,6 +12,7 @@ import jetbrains.buildServer.serverSide.SBuild; import jetbrains.buildServer.serverSide.crypt.EncryptUtil; import jetbrains.buildServer.web.openapi.PluginDescriptor; +import org.apache.commons.lang3.tuple.Pair; import org.apache.log4j.Logger; import org.springframework.web.servlet.ModelAndView; @@ -35,7 +37,7 @@ public ModelAndView getBuildOverviewModelAndView(SBuild build, Map params) { + public Pair buildSpanProcessorAndMeterProvider(BuildPromotion buildPromotion, String endpoint, Map params) { Map headers = new HashMap<>(); params.forEach((k, v) -> { if (k.startsWith(PROPERTY_KEY_HEADERS)) { @@ -45,7 +47,7 @@ public SpanProcessor buildSpanProcessor(BuildPromotion buildPromotion, String en headers.put(name, value); } }); - return buildGrpcSpanProcessor(headers, endpoint); + return Pair.of(buildGrpcSpanProcessor(headers, endpoint), null); } @Override diff --git a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/honeycomb/HoneycombOTELEndpointHandler.java b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/honeycomb/HoneycombOTELEndpointHandler.java index 214c454..6bbd527 100644 --- a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/honeycomb/HoneycombOTELEndpointHandler.java +++ b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/honeycomb/HoneycombOTELEndpointHandler.java @@ -8,10 +8,12 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter; import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporter; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.export.MetricExporter; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.SpanProcessor; import io.opentelemetry.sdk.trace.export.BatchSpanProcessor; +import jetbrains.buildServer.serverSide.BuildPromotion; import io.opentelemetry.semconv.ServiceAttributes; import jetbrains.buildServer.serverSide.BuildPromotion; import jetbrains.buildServer.serverSide.SBuild; @@ -19,11 +21,12 @@ import jetbrains.buildServer.serverSide.crypt.EncryptUtil; import jetbrains.buildServer.serverSide.crypt.RSACipher; import jetbrains.buildServer.web.openapi.PluginDescriptor; +import org.apache.commons.lang3.tuple.Pair; import org.apache.log4j.Logger; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.springframework.web.servlet.ModelAndView; -import javax.annotation.Nullable; import javax.servlet.http.HttpServletRequest; import java.util.Date; import java.util.HashMap; @@ -63,7 +66,7 @@ public ModelAndView getBuildOverviewModelAndView(SBuild build, Map params) { + public Pair buildSpanProcessorAndMeterProvider(BuildPromotion buildPromotion, String endpoint, Map params) { Map headers = new HashMap<>(); //todo: add a setting to say "use classic" or "use environments" headers.put("x-honeycomb-dataset", params.get(PROPERTY_KEY_HONEYCOMB_DATASET)); @@ -86,7 +89,7 @@ private MetricExporter buildMetricsExporter(String endpoint, Map return null; } - private SpanProcessor buildGrpcSpanProcessor( + private Pair buildGrpcSpanProcessor( BuildPromotion buildPromotion, Map headers, String exporterEndpoint, @@ -99,22 +102,24 @@ private SpanProcessor buildGrpcSpanProcessor( AttributeKey.stringKey("teamcity.build_promotion.id"), Long.toString(buildPromotion.getId()), AttributeKey.stringKey("teamcity.node.id"), nodesService.getCurrentNode().getId() )); - var meterProvider = OTELMetrics.getOTELMeterProvider(metricsExporter, serviceNameResource); var spanExporterBuilder = OtlpGrpcSpanExporter.builder(); + spanExporterBuilder.setEndpoint(exporterEndpoint); headers.forEach(spanExporterBuilder::addHeader); - var spanExporter = spanExporterBuilder - .setEndpoint(exporterEndpoint) - .setMeterProvider(meterProvider) - .build(); - - return BatchSpanProcessor.builder(spanExporter) - .setMaxQueueSize(BATCH_SPAN_PROCESSOR_MAX_QUEUE_SIZE) - .setScheduleDelay(BATCH_SPAN_PROCESSOR_MAX_SCHEDULE_DELAY) - .setMaxExportBatchSize(BATCH_SPAN_PROCESSOR_MAX_EXPORT_BATCH_SIZE) - .setMeterProvider(meterProvider) - .build(); + if (meterProvider != null) { + spanExporterBuilder.setMeterProvider(meterProvider); + } + var spanExporter = spanExporterBuilder.build(); + + var batchSpanProcessorBuilder = BatchSpanProcessor.builder(spanExporter); + batchSpanProcessorBuilder.setMaxQueueSize(BATCH_SPAN_PROCESSOR_MAX_QUEUE_SIZE); + batchSpanProcessorBuilder.setScheduleDelay(BATCH_SPAN_PROCESSOR_MAX_SCHEDULE_DELAY); + batchSpanProcessorBuilder.setMaxExportBatchSize(BATCH_SPAN_PROCESSOR_MAX_EXPORT_BATCH_SIZE); + if (meterProvider != null) { + batchSpanProcessorBuilder.setMeterProvider(meterProvider); + } + return Pair.of(batchSpanProcessorBuilder.build(), meterProvider); } @Override diff --git a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/zipkin/ZipkinOTELEndpointHandler.java b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/zipkin/ZipkinOTELEndpointHandler.java index 5f34637..e4206bb 100644 --- a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/zipkin/ZipkinOTELEndpointHandler.java +++ b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/endpoints/zipkin/ZipkinOTELEndpointHandler.java @@ -3,11 +3,13 @@ import com.octopus.teamcity.opentelemetry.server.SetProjectConfigurationSettingsRequest; import com.octopus.teamcity.opentelemetry.server.endpoints.IOTELEndpointHandler; import io.opentelemetry.exporter.zipkin.ZipkinSpanExporter; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.trace.SpanProcessor; import io.opentelemetry.sdk.trace.export.BatchSpanProcessor; import jetbrains.buildServer.serverSide.BuildPromotion; import jetbrains.buildServer.serverSide.SBuild; import jetbrains.buildServer.web.openapi.PluginDescriptor; +import org.apache.commons.lang3.tuple.Pair; import org.jetbrains.annotations.NotNull; import org.springframework.web.servlet.ModelAndView; @@ -35,8 +37,8 @@ public ModelAndView getBuildOverviewModelAndView(SBuild build, Map params) { - return buildZipkinSpanProcessor(endpoint); + public Pair buildSpanProcessorAndMeterProvider(BuildPromotion buildPromotion, String endpoint, Map params) { + return Pair.of(buildZipkinSpanProcessor(endpoint), null); } private SpanProcessor buildZipkinSpanProcessor(String exporterEndpoint) { diff --git a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/HelperPerBuildOTELHelperFactory.java b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/HelperPerBuildOTELHelperFactory.java index f733af0..42e8551 100644 --- a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/HelperPerBuildOTELHelperFactory.java +++ b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/HelperPerBuildOTELHelperFactory.java @@ -1,6 +1,7 @@ package com.octopus.teamcity.opentelemetry.server.helpers; import com.octopus.teamcity.opentelemetry.server.endpoints.OTELEndpointFactory; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.trace.SpanProcessor; import jetbrains.buildServer.serverSide.BuildPromotion; import jetbrains.buildServer.serverSide.ProjectManager; @@ -43,13 +44,14 @@ public OTELHelper getOTELHelper(BuildPromotion buildPromotion) { var params = feature.getParameters(); if (params.get(PROPERTY_KEY_ENABLED).equals("true")) { var endpoint = params.get(PROPERTY_KEY_ENDPOINT); - SpanProcessor spanProcessor; var otelHandler = otelEndpointFactory.getOTELEndpointHandler(params.get(PROPERTY_KEY_SERVICE)); - spanProcessor = otelHandler.buildSpanProcessor(buildPromotion, endpoint, params); + var spanProcessorMeterProviderPair = otelHandler.buildSpanProcessorAndMeterProvider(buildPromotion, endpoint, params); long startTime = System.nanoTime(); - var otelHelper = new OTELHelperImpl(spanProcessor, String.valueOf(buildId)); + var spanProcessor = spanProcessorMeterProviderPair.getLeft(); + var meterProvider = spanProcessorMeterProviderPair.getRight(); + var otelHelper = new OTELHelperImpl(spanProcessor, meterProvider, String.valueOf(buildId)); long endTime = System.nanoTime(); long duration = (endTime - startTime); diff --git a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/OTELHelperImpl.java b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/OTELHelperImpl.java index d48fe4a..7ab0349 100644 --- a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/OTELHelperImpl.java +++ b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/OTELHelperImpl.java @@ -9,6 +9,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.propagation.ContextPropagators; import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.SdkTracerProvider; import io.opentelemetry.sdk.trace.SpanProcessor; @@ -26,8 +27,14 @@ public class OTELHelperImpl implements OTELHelper { private final ConcurrentHashMap spanMap; private final SdkTracerProvider sdkTracerProvider; private final String helperName; + @Nullable + private final SdkMeterProvider meterProvider; - public OTELHelperImpl(SpanProcessor spanProcessor, String helperName) { + public OTELHelperImpl( + SpanProcessor spanProcessor, + @Nullable + SdkMeterProvider meterProvider, + String helperName) { this.helperName = helperName; Resource serviceNameResource = Resource .create(Attributes.of(ServiceAttributes.SERVICE_NAME, PluginConstants.SERVICE_NAME)); @@ -41,6 +48,7 @@ public OTELHelperImpl(SpanProcessor spanProcessor, String helperName) { .build(); this.tracer = this.openTelemetry.getTracer(PluginConstants.TRACER_INSTRUMENTATION_NAME); this.spanMap = new ConcurrentHashMap<>(); + this.meterProvider = meterProvider; } @Override @@ -92,6 +100,8 @@ public void release(String helperName) { this.sdkTracerProvider.forceFlush(); this.sdkTracerProvider.close(); + if (this.meterProvider != null) + this.meterProvider.close(); this.spanMap.clear(); } } diff --git a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/OTELMetrics.java b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/OTELMetrics.java index 8c4234d..b5c6c22 100644 --- a/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/OTELMetrics.java +++ b/server/src/main/java/com/octopus/teamcity/opentelemetry/server/helpers/OTELMetrics.java @@ -1,7 +1,5 @@ package com.octopus.teamcity.opentelemetry.server.helpers; -import io.opentelemetry.api.GlobalOpenTelemetry; -import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.export.MetricExporter; import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; @@ -9,40 +7,20 @@ import javax.annotation.Nullable; import java.time.Duration; -import java.util.concurrent.atomic.AtomicBoolean; public class OTELMetrics { - private static final AtomicBoolean metricsConfigured = new AtomicBoolean(false); - private static SdkMeterProvider sdkMeterProvider; - - // this is not quite right... - // it's using project level settings (which can be different across projects) - // but setting up a global configuration. - // likely need to refactor the config so that it has metrics configuration as global - + @Nullable public static SdkMeterProvider getOTELMeterProvider(@Nullable MetricExporter metricExporter, Resource serviceNameResource) { - if (metricsConfigured.get()) return sdkMeterProvider; - metricsConfigured.set(true); + if (metricExporter == null) return null; - var meterProviderBuilder = SdkMeterProvider.builder() - .setResource(Resource.getDefault().merge(serviceNameResource)); - if (metricExporter != null) { - var providedMetricExporterBuilder = PeriodicMetricReader.builder(metricExporter) - .setInterval(Duration.ofSeconds(10)) - .build(); - meterProviderBuilder - .registerMetricReader(providedMetricExporterBuilder); - } - sdkMeterProvider = meterProviderBuilder.build(); - var globalOpenTelemetry = OpenTelemetrySdk.builder() - .setMeterProvider(sdkMeterProvider) + var providedMetricExporter = PeriodicMetricReader.builder(metricExporter) + .setInterval(Duration.ofSeconds(10)) .build(); - GlobalOpenTelemetry.set(globalOpenTelemetry); - - // Shutdown hooks to close resources properly - Runtime.getRuntime().addShutdownHook(new Thread(sdkMeterProvider::close)); - return sdkMeterProvider; + return SdkMeterProvider.builder() + .setResource(Resource.getDefault().merge(serviceNameResource)) + .registerMetricReader(providedMetricExporter) + .build(); } } diff --git a/server/src/test/java/com/octopus/teamcity/opentelemetry/server/OTELHelperTest.java b/server/src/test/java/com/octopus/teamcity/opentelemetry/server/OTELHelperTest.java index a681496..48a228a 100644 --- a/server/src/test/java/com/octopus/teamcity/opentelemetry/server/OTELHelperTest.java +++ b/server/src/test/java/com/octopus/teamcity/opentelemetry/server/OTELHelperTest.java @@ -32,7 +32,7 @@ class OTELHelperTest { @BeforeEach void setUp() { GlobalOpenTelemetry.resetForTest(); - this.otelHelper = new OTELHelperImpl(mock(SpanProcessor.class, RETURNS_DEEP_STUBS), "helperNamr"); + this.otelHelper = new OTELHelperImpl(mock(SpanProcessor.class, RETURNS_DEEP_STUBS), null, "helperNamr"); } diff --git a/server/src/test/java/com/octopus/teamcity/opentelemetry/server/TeamCityBuildListenerTest.java b/server/src/test/java/com/octopus/teamcity/opentelemetry/server/TeamCityBuildListenerTest.java index edab60a..9bdc3e7 100644 --- a/server/src/test/java/com/octopus/teamcity/opentelemetry/server/TeamCityBuildListenerTest.java +++ b/server/src/test/java/com/octopus/teamcity/opentelemetry/server/TeamCityBuildListenerTest.java @@ -33,7 +33,7 @@ class TeamCityBuildListenerTest { @BeforeEach void setUp(@Mock EventDispatcher buildServerListenerEventDispatcher) { GlobalOpenTelemetry.resetForTest(); - this.otelHelper = new OTELHelperImpl(mock(SpanProcessor.class, RETURNS_DEEP_STUBS), "helper"); + this.otelHelper = new OTELHelperImpl(mock(SpanProcessor.class, RETURNS_DEEP_STUBS), null, "helper"); this.factory = mock(OTELHelperFactory.class, RETURNS_DEEP_STUBS); var buildStorageManager = mock(BuildStorageManager.class, RETURNS_DEEP_STUBS);