From a6930e8343534659c5b177d6bfa308d1e77a3500 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Tue, 26 Sep 2023 20:11:16 +0530 Subject: [PATCH 01/19] Add Telemetry metrics framework Signed-off-by: Gagan Juneja --- .../opensearch/telemetry/metrics/Counter.java | 31 +++ .../metrics/DefaultMetricsRegistry.java | 42 ++++ .../telemetry/metrics/MetricsRegistry.java | 35 +++ .../telemetry/metrics/MetricsTelemetry.java | 21 +- .../telemetry/metrics/noop/NoopCounter.java | 35 +++ .../metrics/noop/NoopMetricsRegistry.java | 42 ++++ .../telemetry/metrics/noop/package-info.java | 12 ++ .../metrics/DefaultMetricsRegistryTests.java | 51 +++++ plugins/telemetry-otel/build.gradle | 22 +- ...-extension-incubator-1.30.1-alpha.jar.sha1 | 1 + ...ntelemetry-extension-incubator-LICENSE.txt | 202 ++++++++++++++++++ ...entelemetry-extension-incubator-NOTICE.txt | 0 .../OTelAttributesConverter.java | 6 +- .../telemetry/OTelTelemetryPlugin.java | 11 +- .../telemetry/OTelTelemetrySettings.java | 10 + .../telemetry/metrics/OTelCounter.java | 40 ++++ .../metrics/OTelMetricsTelemetry.java | 74 +++++++ .../telemetry/metrics/OTelUpDownCounter.java | 40 ++++ .../telemetry/metrics/package-info.java | 12 ++ .../tracing/OTelResourceProvider.java | 35 ++- .../tracing/OTelTracingTelemetry.java | 1 + .../plugin-metadata/plugin-security.policy | 1 + .../telemetry/OTelTelemetryPluginTests.java | 26 ++- .../metrics/OTelMetricsTelemetryTests.java | 104 +++++++++ .../tracing/OTelAttributesConverterTests.java | 1 + .../common/settings/ClusterSettings.java | 6 +- .../main/java/org/opensearch/node/Node.java | 12 ++ .../telemetry/TelemetrySettings.java | 28 +++ .../metrics/MetricsRegistryFactory.java | 81 +++++++ .../metrics/NoopMetricsRegistryFactory.java | 36 ++++ .../telemetry/metrics/WrappedCounter.java | 51 +++++ .../metrics/WrappedMetricsRegistry.java | 49 +++++ .../telemetry/metrics/package-info.java | 12 ++ .../metrics/MetricsRegistryFactoryTests.java | 75 +++++++ .../tracing/metrics/WrappedCounterTests.java | 65 ++++++ .../metrics/WrappedMetricsRegistryTests.java | 54 +++++ .../test/telemetry/MockTelemetry.java | 15 ++ 37 files changed, 1301 insertions(+), 38 deletions(-) create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Counter.java create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistry.java create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistry.java create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopCounter.java create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopMetricsRegistry.java create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/package-info.java create mode 100644 libs/telemetry/src/test/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistryTests.java create mode 100644 plugins/telemetry-otel/licenses/opentelemetry-extension-incubator-1.30.1-alpha.jar.sha1 create mode 100644 plugins/telemetry-otel/licenses/opentelemetry-extension-incubator-LICENSE.txt create mode 100644 plugins/telemetry-otel/licenses/opentelemetry-extension-incubator-NOTICE.txt rename plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/{tracing => }/OTelAttributesConverter.java (89%) create mode 100644 plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelCounter.java create mode 100644 plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java create mode 100644 plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelUpDownCounter.java create mode 100644 plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/package-info.java create mode 100644 plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java create mode 100644 server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java create mode 100644 server/src/main/java/org/opensearch/telemetry/metrics/NoopMetricsRegistryFactory.java create mode 100644 server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java create mode 100644 server/src/main/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistry.java create mode 100644 server/src/main/java/org/opensearch/telemetry/metrics/package-info.java create mode 100644 server/src/test/java/org/opensearch/telemetry/tracing/metrics/MetricsRegistryFactoryTests.java create mode 100644 server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java create mode 100644 server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedMetricsRegistryTests.java diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Counter.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Counter.java new file mode 100644 index 0000000000000..3c77087a939a1 --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Counter.java @@ -0,0 +1,31 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics; + +import org.opensearch.telemetry.tracing.attributes.Attributes; + +/** + * Counter adds the value to the existing metric. + */ +public interface Counter { + + /** + * add value. + * @param value value to be added. + */ + void add(double value); + + /** + * add value along with the attributes. + * @param value value to be added. + * @param attributes attributes/dimensions of the metric. + */ + void add(double value, Attributes attributes); + +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistry.java new file mode 100644 index 0000000000000..4b3b0bd681c47 --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistry.java @@ -0,0 +1,42 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics; + +import java.io.Closeable; +import java.io.IOException; + +/** + * Default implementation for {@link MetricsRegistry} + */ +public class DefaultMetricsRegistry implements MetricsRegistry { + private final MetricsTelemetry metricsTelemetry; + + /** + * Constructor + * @param metricsTelemetry metrics telemetry. + */ + public DefaultMetricsRegistry(MetricsTelemetry metricsTelemetry) { + this.metricsTelemetry = metricsTelemetry; + } + + @Override + public Counter createCounter(String name, String description, String unit) { + return metricsTelemetry.createCounter(name, description, unit); + } + + @Override + public Counter createUpDownCounter(String name, String description, String unit) { + return metricsTelemetry.createUpDownCounter(name, description, unit); + } + + @Override + public void close() throws IOException { + ((Closeable) metricsTelemetry).close(); + } +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistry.java new file mode 100644 index 0000000000000..b5f26ae822060 --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistry.java @@ -0,0 +1,35 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics; + +import java.io.Closeable; + +/** + * MetricsRegistry helps in creating the metric instruments. + */ +public interface MetricsRegistry extends Closeable { + + /** + * Creates the counter. + * @param name name of the counter. + * @param description any description about the metric. + * @param unit unit of the metric. + * @return counter. + */ + Counter createCounter(String name, String description, String unit); + + /** + * Creates the upDown counter. + * @param name name of the upDown counter. + * @param description any description about the metric. + * @param unit unit of the metric. + * @return counter. + */ + Counter createUpDownCounter(String name, String description, String unit); +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java index 0bf9482fe58d8..d0c999e7c5c52 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java @@ -10,12 +10,31 @@ import org.opensearch.common.annotation.ExperimentalApi; +import java.io.Closeable; + /** * Interface for metrics telemetry providers * * @opensearch.experimental */ @ExperimentalApi -public interface MetricsTelemetry { +public interface MetricsTelemetry extends Closeable { + + /** + * Creates the counter. + * @param name name of the counter. + * @param description any description about the metric. + * @param unit unit of the metric. + * @return counter + */ + Counter createCounter(String name, String description, String unit); + /** + * Creates the upDown counter. + * @param name name of the upDown counter. + * @param description any description about the metric. + * @param unit unit of the metric. + * @return upDownCounter. + */ + Counter createUpDownCounter(String name, String description, String unit); } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopCounter.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopCounter.java new file mode 100644 index 0000000000000..fc1529157b4a7 --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopCounter.java @@ -0,0 +1,35 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics.noop; + +import org.opensearch.telemetry.metrics.Counter; +import org.opensearch.telemetry.tracing.attributes.Attributes; + +/** + * No-op {@link Counter} + */ +public class NoopCounter implements Counter { + + /** + * No-op Counter instance + */ + public final static NoopCounter INSTANCE = new NoopCounter(); + + private NoopCounter() {} + + @Override + public void add(double value) { + + } + + @Override + public void add(double value, Attributes attributes) { + + } +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopMetricsRegistry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopMetricsRegistry.java new file mode 100644 index 0000000000000..1836ec8729c3e --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopMetricsRegistry.java @@ -0,0 +1,42 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics.noop; + +import org.opensearch.telemetry.metrics.Counter; +import org.opensearch.telemetry.metrics.MetricsRegistry; + +import java.io.IOException; + +/** + *No-op {@link MetricsRegistry} + */ +public class NoopMetricsRegistry implements MetricsRegistry { + + /** + * No-op Meter instance + */ + public final static NoopMetricsRegistry INSTANCE = new NoopMetricsRegistry(); + + private NoopMetricsRegistry() {} + + @Override + public Counter createCounter(String name, String description, String unit) { + return NoopCounter.INSTANCE; + } + + @Override + public Counter createUpDownCounter(String name, String description, String unit) { + return NoopCounter.INSTANCE; + } + + @Override + public void close() throws IOException { + + } +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/package-info.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/package-info.java new file mode 100644 index 0000000000000..89380bb096538 --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Contains metrics related classes + */ +package org.opensearch.telemetry.metrics.noop; diff --git a/libs/telemetry/src/test/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistryTests.java b/libs/telemetry/src/test/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistryTests.java new file mode 100644 index 0000000000000..6171641db5f07 --- /dev/null +++ b/libs/telemetry/src/test/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistryTests.java @@ -0,0 +1,51 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics; + +import org.opensearch.test.OpenSearchTestCase; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class DefaultMetricsRegistryTests extends OpenSearchTestCase { + + private MetricsTelemetry metricsTelemetry; + private DefaultMetricsRegistry defaultMeterRegistry; + + @Override + public void setUp() throws Exception { + super.setUp(); + metricsTelemetry = mock(MetricsTelemetry.class); + defaultMeterRegistry = new DefaultMetricsRegistry(metricsTelemetry); + } + + public void testCounter() { + Counter mockCounter = mock(Counter.class); + when(defaultMeterRegistry.createCounter(any(String.class), any(String.class), any(String.class))).thenReturn(mockCounter); + Counter counter = defaultMeterRegistry.createCounter( + "org.opensearch.telemetry.metrics.DefaultMeterRegistryTests.testCounter", + "test counter", + "1" + ); + assertSame(mockCounter, counter); + } + + public void testUpDownCounter() { + Counter mockCounter = mock(Counter.class); + when(defaultMeterRegistry.createUpDownCounter(any(String.class), any(String.class), any(String.class))).thenReturn(mockCounter); + Counter counter = defaultMeterRegistry.createUpDownCounter( + "org.opensearch.telemetry.metrics.DefaultMeterRegistryTests.testUpDownCounter", + "test up-down counter", + "1" + ); + assertSame(mockCounter, counter); + } + +} diff --git a/plugins/telemetry-otel/build.gradle b/plugins/telemetry-otel/build.gradle index 45c9f522c09d8..04fff20947b4f 100644 --- a/plugins/telemetry-otel/build.gradle +++ b/plugins/telemetry-otel/build.gradle @@ -37,6 +37,7 @@ dependencies { runtimeOnly "com.squareup.okhttp3:okhttp:4.11.0" runtimeOnly "com.squareup.okio:okio-jvm:3.5.0" runtimeOnly "io.opentelemetry:opentelemetry-exporter-sender-okhttp:${versions.opentelemetry}" + api "io.opentelemetry:opentelemetry-extension-incubator:${versions.opentelemetry}-alpha" testImplementation "io.opentelemetry:opentelemetry-sdk-testing:${versions.opentelemetry}" } @@ -80,29 +81,12 @@ thirdPartyAudit { 'io.opentelemetry.api.events.EventEmitter', 'io.opentelemetry.api.events.EventEmitterBuilder', 'io.opentelemetry.api.events.EventEmitterProvider', - 'io.opentelemetry.extension.incubator.metrics.ExtendedDoubleHistogramBuilder', - 'io.opentelemetry.extension.incubator.metrics.ExtendedLongHistogramBuilder', 'io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties', 'io.opentelemetry.sdk.autoconfigure.spi.logs.ConfigurableLogRecordExporterProvider', 'io.opentelemetry.sdk.autoconfigure.spi.metrics.ConfigurableMetricExporterProvider', 'io.opentelemetry.sdk.autoconfigure.spi.traces.ConfigurableSpanExporterProvider', - 'io.opentelemetry.extension.incubator.metrics.DoubleCounterAdviceConfigurer', - 'io.opentelemetry.extension.incubator.metrics.DoubleGauge', - 'io.opentelemetry.extension.incubator.metrics.DoubleGaugeAdviceConfigurer', - 'io.opentelemetry.extension.incubator.metrics.DoubleHistogramAdviceConfigurer', - 'io.opentelemetry.extension.incubator.metrics.DoubleUpDownCounterAdviceConfigurer', - 'io.opentelemetry.extension.incubator.metrics.ExtendedDoubleCounterBuilder', - 'io.opentelemetry.extension.incubator.metrics.ExtendedDoubleGaugeBuilder', - 'io.opentelemetry.extension.incubator.metrics.ExtendedDoubleUpDownCounterBuilder', - 'io.opentelemetry.extension.incubator.metrics.ExtendedLongCounterBuilder', - 'io.opentelemetry.extension.incubator.metrics.ExtendedLongGaugeBuilder', - 'io.opentelemetry.extension.incubator.metrics.ExtendedLongUpDownCounterBuilder', - 'io.opentelemetry.extension.incubator.metrics.LongCounterAdviceConfigurer', - 'io.opentelemetry.extension.incubator.metrics.LongGauge', - 'io.opentelemetry.extension.incubator.metrics.LongGaugeAdviceConfigurer', - 'io.opentelemetry.extension.incubator.metrics.LongHistogramAdviceConfigurer', - 'io.opentelemetry.extension.incubator.metrics.LongUpDownCounterAdviceConfigurer', - 'kotlin.io.path.PathsKt' + 'kotlin.io.path.PathsKt', + 'io.opentelemetry.sdk.autoconfigure.spi.traces.ConfigurableSpanExporterProvider' ) } diff --git a/plugins/telemetry-otel/licenses/opentelemetry-extension-incubator-1.30.1-alpha.jar.sha1 b/plugins/telemetry-otel/licenses/opentelemetry-extension-incubator-1.30.1-alpha.jar.sha1 new file mode 100644 index 0000000000000..bde43937e82e4 --- /dev/null +++ b/plugins/telemetry-otel/licenses/opentelemetry-extension-incubator-1.30.1-alpha.jar.sha1 @@ -0,0 +1 @@ +bfcea9bd71f97dd4e8a4f92c15ba5659fb07ff05 \ No newline at end of file diff --git a/plugins/telemetry-otel/licenses/opentelemetry-extension-incubator-LICENSE.txt b/plugins/telemetry-otel/licenses/opentelemetry-extension-incubator-LICENSE.txt new file mode 100644 index 0000000000000..d645695673349 --- /dev/null +++ b/plugins/telemetry-otel/licenses/opentelemetry-extension-incubator-LICENSE.txt @@ -0,0 +1,202 @@ + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/plugins/telemetry-otel/licenses/opentelemetry-extension-incubator-NOTICE.txt b/plugins/telemetry-otel/licenses/opentelemetry-extension-incubator-NOTICE.txt new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelAttributesConverter.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelAttributesConverter.java similarity index 89% rename from plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelAttributesConverter.java rename to plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelAttributesConverter.java index 4d0966e6b5185..b6c8dfda9a27b 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelAttributesConverter.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelAttributesConverter.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.telemetry.tracing; +package org.opensearch.telemetry; import java.util.Locale; @@ -16,7 +16,7 @@ /** * Converts {@link org.opensearch.telemetry.tracing.attributes.Attributes} to OTel {@link Attributes} */ -final class OTelAttributesConverter { +public final class OTelAttributesConverter { /** * Constructor. @@ -28,7 +28,7 @@ private OTelAttributesConverter() {} * @param attributes attributes * @return otel attributes. */ - static Attributes convert(org.opensearch.telemetry.tracing.attributes.Attributes attributes) { + public static Attributes convert(org.opensearch.telemetry.tracing.attributes.Attributes attributes) { AttributesBuilder attributesBuilder = Attributes.builder(); if (attributes != null) { attributes.getAttributesMap().forEach((x, y) -> addSpanAttribute(x, y, attributesBuilder)); diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java index 1af88196e3727..97ed7303643cb 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java @@ -12,7 +12,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.plugins.Plugin; import org.opensearch.plugins.TelemetryPlugin; -import org.opensearch.telemetry.metrics.MetricsTelemetry; +import org.opensearch.telemetry.metrics.OTelMetricsTelemetry; import org.opensearch.telemetry.tracing.OTelResourceProvider; import org.opensearch.telemetry.tracing.OTelTelemetry; import org.opensearch.telemetry.tracing.OTelTracingTelemetry; @@ -21,6 +21,8 @@ import java.util.List; import java.util.Optional; +import io.opentelemetry.api.OpenTelemetry; + /** * Telemetry plugin based on Otel */ @@ -44,7 +46,8 @@ public List> getSettings() { OTelTelemetrySettings.TRACER_EXPORTER_BATCH_SIZE_SETTING, OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING, OTelTelemetrySettings.TRACER_EXPORTER_MAX_QUEUE_SIZE_SETTING, - OTelTelemetrySettings.OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING + OTelTelemetrySettings.OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING, + OTelTelemetrySettings.METRICS_PUBLISH_INTERVAL_SETTING ); } @@ -59,8 +62,8 @@ public String getName() { } private Telemetry telemetry(TelemetrySettings telemetrySettings) { - return new OTelTelemetry(new OTelTracingTelemetry(OTelResourceProvider.get(telemetrySettings, settings)), new MetricsTelemetry() { - }); + final OpenTelemetry openTelemetry = OTelResourceProvider.get(telemetrySettings, settings); + return new OTelTelemetry(new OTelTracingTelemetry(openTelemetry), new OTelMetricsTelemetry(openTelemetry)); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java index 59c87cca22986..4d1174cddc8b2 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java @@ -60,6 +60,16 @@ private OTelTelemetrySettings() {} Setting.Property.Final ); + /** + * metrics publish interval in seconds. + */ + public static final Setting METRICS_PUBLISH_INTERVAL_SETTING = Setting.timeSetting( + "telemetry.otel.metrics.publish.interval", + TimeValue.timeValueSeconds(60), + Setting.Property.NodeScope, + Setting.Property.Final + ); + /** * Span Exporter type setting. */ diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelCounter.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelCounter.java new file mode 100644 index 0000000000000..f50a65d8417ad --- /dev/null +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelCounter.java @@ -0,0 +1,40 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics; + +import org.opensearch.telemetry.OTelAttributesConverter; +import org.opensearch.telemetry.tracing.attributes.Attributes; + +import io.opentelemetry.api.metrics.DoubleCounter; + +/** + * OTel Counter + */ +public class OTelCounter implements Counter { + + private final DoubleCounter otelDoubleCounter; + + /** + * Constructor + * @param otelDoubleCounter delegate counter. + */ + public OTelCounter(DoubleCounter otelDoubleCounter) { + this.otelDoubleCounter = otelDoubleCounter; + } + + @Override + public void add(double value) { + otelDoubleCounter.add(value); + } + + @Override + public void add(double value, Attributes attributes) { + otelDoubleCounter.add(value, OTelAttributesConverter.convert(attributes)); + } +} diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java new file mode 100644 index 0000000000000..3c9d0e239696b --- /dev/null +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java @@ -0,0 +1,74 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import java.io.Closeable; +import java.io.IOException; +import java.security.AccessController; +import java.security.PrivilegedAction; + +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.metrics.DoubleCounter; +import io.opentelemetry.api.metrics.DoubleUpDownCounter; +import io.opentelemetry.api.metrics.Meter; + +/** + * OTel implementation for {@link MetricsTelemetry} + */ +public class OTelMetricsTelemetry implements MetricsTelemetry { + private static final Logger logger = LogManager.getLogger(OTelMetricsTelemetry.class); + private final OpenTelemetry openTelemetry; + private final Meter otelMeter; + + /** + * Constructor. + * @param openTelemetry telemetry. + */ + public OTelMetricsTelemetry(OpenTelemetry openTelemetry) { + this.openTelemetry = openTelemetry; + this.otelMeter = openTelemetry.getMeter("os-meter"); + } + + @Override + public Counter createCounter(String name, String description, String unit) { + DoubleCounter doubleCounter = AccessController.doPrivileged( + (PrivilegedAction) () -> otelMeter.counterBuilder(name) + .setUnit(unit) + .setDescription(description) + .ofDoubles() + .build() + ); + return new OTelCounter(doubleCounter); + } + + @Override + public Counter createUpDownCounter(String name, String description, String unit) { + DoubleUpDownCounter doubleUpDownCounter = AccessController.doPrivileged( + (PrivilegedAction) () -> otelMeter.upDownCounterBuilder(name) + .setUnit(unit) + .setDescription(description) + .ofDoubles() + .build() + ); + return new OTelUpDownCounter(doubleUpDownCounter); + } + + @Override + public void close() { + // There is no harm closing the openTelemetry multiple times. + try { + ((Closeable) openTelemetry).close(); + } catch (IOException e) { + logger.warn("Error while closing Opentelemetry", e); + } + } +} diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelUpDownCounter.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelUpDownCounter.java new file mode 100644 index 0000000000000..44b24e5dbc808 --- /dev/null +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelUpDownCounter.java @@ -0,0 +1,40 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics; + +import org.opensearch.telemetry.OTelAttributesConverter; +import org.opensearch.telemetry.tracing.attributes.Attributes; + +import io.opentelemetry.api.metrics.DoubleUpDownCounter; + +/** + * OTel Counter + */ +public class OTelUpDownCounter implements Counter { + + private final DoubleUpDownCounter doubleUpDownCounter; + + /** + * Constructor + * @param doubleUpDownCounter delegate counter. + */ + public OTelUpDownCounter(DoubleUpDownCounter doubleUpDownCounter) { + this.doubleUpDownCounter = doubleUpDownCounter; + } + + @Override + public void add(double value) { + doubleUpDownCounter.add(value); + } + + @Override + public void add(double value, Attributes attributes) { + doubleUpDownCounter.add(value, OTelAttributesConverter.convert(attributes)); + } +} diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/package-info.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/package-info.java new file mode 100644 index 0000000000000..803c159eb201a --- /dev/null +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * This package contains classes needed for tracing requests. + */ +package org.opensearch.telemetry.metrics; diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java index fe05cc8bb7a41..0ce7de772fe7d 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java @@ -22,7 +22,11 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; import io.opentelemetry.context.propagation.ContextPropagators; +import io.opentelemetry.exporter.logging.LoggingMetricExporter; import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.metrics.data.AggregationTemporality; +import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.SdkTracerProvider; import io.opentelemetry.sdk.trace.export.BatchSpanProcessor; @@ -30,6 +34,7 @@ import io.opentelemetry.sdk.trace.samplers.Sampler; import io.opentelemetry.semconv.resource.attributes.ResourceAttributes; +import static org.opensearch.telemetry.OTelTelemetrySettings.METRICS_PUBLISH_INTERVAL_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_BATCH_SIZE_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_MAX_QUEUE_SIZE_SETTING; @@ -67,13 +72,37 @@ public static OpenTelemetry get(TelemetrySettings telemetrySettings, Settings se */ public static OpenTelemetry get(Settings settings, SpanExporter spanExporter, ContextPropagators contextPropagators, Sampler sampler) { Resource resource = Resource.create(Attributes.of(ResourceAttributes.SERVICE_NAME, "OpenSearch")); - SdkTracerProvider sdkTracerProvider = SdkTracerProvider.builder() + SdkTracerProvider sdkTracerProvider = createSdkTracerProvider(settings, spanExporter, sampler, resource); + SdkMeterProvider sdkMeterProvider = createSdkMetricProvider(settings, resource); + return OpenTelemetrySdk.builder() + .setTracerProvider(sdkTracerProvider) + .setMeterProvider(sdkMeterProvider) + .setPropagators(contextPropagators) + .buildAndRegisterGlobal(); + } + + private static SdkMeterProvider createSdkMetricProvider(Settings settings, Resource resource) { + return SdkMeterProvider.builder() + .setResource(resource) + .registerMetricReader( + PeriodicMetricReader.builder(LoggingMetricExporter.create(AggregationTemporality.DELTA)) + .setInterval(METRICS_PUBLISH_INTERVAL_SETTING.get(settings).getSeconds(), TimeUnit.SECONDS) + .build() + ) + .build(); + } + + private static SdkTracerProvider createSdkTracerProvider( + Settings settings, + SpanExporter spanExporter, + Sampler sampler, + Resource resource + ) { + return SdkTracerProvider.builder() .addSpanProcessor(spanProcessor(settings, spanExporter)) .setResource(resource) .setSampler(sampler) .build(); - - return OpenTelemetrySdk.builder().setTracerProvider(sdkTracerProvider).setPropagators(contextPropagators).buildAndRegisterGlobal(); } private static BatchSpanProcessor spanProcessor(Settings settings, SpanExporter spanExporter) { diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index 53066ad4ad444..726205f8f8573 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.telemetry.OTelAttributesConverter; import java.io.Closeable; import java.io.IOException; diff --git a/plugins/telemetry-otel/src/main/plugin-metadata/plugin-security.policy b/plugins/telemetry-otel/src/main/plugin-metadata/plugin-security.policy index 726db3d3f4700..9d529ed5a2a56 100644 --- a/plugins/telemetry-otel/src/main/plugin-metadata/plugin-security.policy +++ b/plugins/telemetry-otel/src/main/plugin-metadata/plugin-security.policy @@ -11,6 +11,7 @@ grant { permission java.lang.RuntimePermission "accessDeclaredMembers"; permission java.net.NetPermission "getProxySelector"; permission java.net.SocketPermission "*", "connect,resolve"; + permission java.util.PropertyPermission "*", "read,write"; }; diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java index 8c2b5d14733e2..46100414e86c8 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java @@ -12,6 +12,8 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; +import org.opensearch.telemetry.metrics.MetricsTelemetry; +import org.opensearch.telemetry.metrics.OTelMetricsTelemetryTests; import org.opensearch.telemetry.tracing.OTelTracingTelemetry; import org.opensearch.telemetry.tracing.TracingTelemetry; import org.opensearch.test.OpenSearchTestCase; @@ -25,44 +27,54 @@ import java.util.Set; import static org.opensearch.telemetry.OTelTelemetryPlugin.OTEL_TRACER_NAME; +import static org.opensearch.telemetry.OTelTelemetrySettings.METRICS_PUBLISH_INTERVAL_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_BATCH_SIZE_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_MAX_QUEUE_SIZE_SETTING; +import static org.opensearch.telemetry.TelemetrySettings.METRICS_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_PROBABILITY; public class OTelTelemetryPluginTests extends OpenSearchTestCase { - private OTelTelemetryPlugin oTelTracerModulePlugin; + private OTelTelemetryPlugin oTelTelemetryModulePlugin; private Optional telemetry; private TracingTelemetry tracingTelemetry; + private MetricsTelemetry metricsTelemetry; + @Before public void setup() { // TRACER_EXPORTER_DELAY_SETTING should always be less than 10 seconds because // io.opentelemetry.sdk.OpenTelemetrySdk.close waits only for 10 seconds for shutdown to complete. Settings settings = Settings.builder().put(TRACER_EXPORTER_DELAY_SETTING.getKey(), "1s").build(); - oTelTracerModulePlugin = new OTelTelemetryPlugin(settings); - telemetry = oTelTracerModulePlugin.getTelemetry( - new TelemetrySettings(Settings.EMPTY, new ClusterSettings(settings, Set.of(TRACER_ENABLED_SETTING, TRACER_SAMPLER_PROBABILITY))) + oTelTelemetryModulePlugin = new OTelTelemetryPlugin(settings); + telemetry = oTelTelemetryModulePlugin.getTelemetry( + new TelemetrySettings( + Settings.EMPTY, + new ClusterSettings(settings, Set.of(TRACER_ENABLED_SETTING, TRACER_SAMPLER_PROBABILITY, METRICS_ENABLED_SETTING)) + ) ); tracingTelemetry = telemetry.get().getTracingTelemetry(); + metricsTelemetry = telemetry.get().getMetricsTelemetry(); } public void testGetTelemetry() { Set> allTracerSettings = new HashSet<>(); ClusterSettings.FEATURE_FLAGGED_CLUSTER_SETTINGS.get(List.of(FeatureFlags.TELEMETRY)).stream().forEach((allTracerSettings::add)); - assertEquals(OTEL_TRACER_NAME, oTelTracerModulePlugin.getName()); + assertEquals(OTEL_TRACER_NAME, oTelTelemetryModulePlugin.getName()); assertTrue(tracingTelemetry instanceof OTelTracingTelemetry); + assertTrue(metricsTelemetry instanceof OTelMetricsTelemetryTests); assertEquals( Arrays.asList( TRACER_EXPORTER_BATCH_SIZE_SETTING, TRACER_EXPORTER_DELAY_SETTING, TRACER_EXPORTER_MAX_QUEUE_SIZE_SETTING, - OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING + OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING, + METRICS_PUBLISH_INTERVAL_SETTING ), - oTelTracerModulePlugin.getSettings() + oTelTelemetryModulePlugin.getSettings() ); } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java new file mode 100644 index 0000000000000..3bd306f46dbce --- /dev/null +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java @@ -0,0 +1,104 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics; + +import org.opensearch.telemetry.OTelAttributesConverter; +import org.opensearch.telemetry.tracing.attributes.Attributes; +import org.opensearch.test.OpenSearchTestCase; + +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.metrics.DoubleCounter; +import io.opentelemetry.api.metrics.DoubleCounterBuilder; +import io.opentelemetry.api.metrics.DoubleUpDownCounter; +import io.opentelemetry.api.metrics.DoubleUpDownCounterBuilder; +import io.opentelemetry.api.metrics.LongCounterBuilder; +import io.opentelemetry.api.metrics.LongUpDownCounterBuilder; +import io.opentelemetry.api.metrics.Meter; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class OTelMetricsTelemetryTests extends OpenSearchTestCase { + + public void testCounter() { + String counterName = "test-counter"; + String description = "test"; + String unit = "1"; + OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + Meter mockMeter = mock(Meter.class); + DoubleCounter mockOTelDoubleCounter = mock(DoubleCounter.class); + LongCounterBuilder mockOTelLongCounterBuilder = mock(LongCounterBuilder.class); + DoubleCounterBuilder mockOTelDoubleCounterBuilder = mock(DoubleCounterBuilder.class); + + when(mockOpenTelemetry.getMeter("os-meter")).thenReturn(mockMeter); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry); + when(mockMeter.counterBuilder(counterName)).thenReturn(mockOTelLongCounterBuilder); + when(mockOTelLongCounterBuilder.setDescription(description)).thenReturn(mockOTelLongCounterBuilder); + when(mockOTelLongCounterBuilder.setUnit(unit)).thenReturn(mockOTelLongCounterBuilder); + when(mockOTelLongCounterBuilder.ofDoubles()).thenReturn(mockOTelDoubleCounterBuilder); + when(mockOTelDoubleCounterBuilder.build()).thenReturn(mockOTelDoubleCounter); + + Counter counter = metricsTelemetry.createCounter(counterName, description, unit); + counter.add(1.0); + verify(mockOTelDoubleCounter).add(1.0); + Attributes attributes = Attributes.create().addAttribute("test", "test"); + counter.add(2.0, attributes); + verify(mockOTelDoubleCounter).add(2.0, OTelAttributesConverter.convert(attributes)); + } + + public void testCounterNegativeValue() { + String counterName = "test-counter"; + String description = "test"; + String unit = "1"; + OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + Meter mockMeter = mock(Meter.class); + DoubleCounter mockOTelDoubleCounter = mock(DoubleCounter.class); + LongCounterBuilder mockOTelLongCounterBuilder = mock(LongCounterBuilder.class); + DoubleCounterBuilder mockOTelDoubleCounterBuilder = mock(DoubleCounterBuilder.class); + + when(mockOpenTelemetry.getMeter("os-meter")).thenReturn(mockMeter); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry); + when(mockMeter.counterBuilder(counterName)).thenReturn(mockOTelLongCounterBuilder); + when(mockOTelLongCounterBuilder.setDescription(description)).thenReturn(mockOTelLongCounterBuilder); + when(mockOTelLongCounterBuilder.setUnit(unit)).thenReturn(mockOTelLongCounterBuilder); + when(mockOTelLongCounterBuilder.ofDoubles()).thenReturn(mockOTelDoubleCounterBuilder); + when(mockOTelDoubleCounterBuilder.build()).thenReturn(mockOTelDoubleCounter); + + Counter counter = metricsTelemetry.createCounter(counterName, description, unit); + counter.add(-1.0); + verify(mockOTelDoubleCounter).add(1.0); + } + + public void testUpDownCounter() { + String counterName = "test-counter"; + String description = "test"; + String unit = "1"; + OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + Meter mockMeter = mock(Meter.class); + DoubleUpDownCounter mockOTelUpDownDoubleCounter = mock(DoubleUpDownCounter.class); + LongUpDownCounterBuilder mockOTelLongUpDownCounterBuilder = mock(LongUpDownCounterBuilder.class); + DoubleUpDownCounterBuilder mockOTelDoubleUpDownCounterBuilder = mock(DoubleUpDownCounterBuilder.class); + + when(mockOpenTelemetry.getMeter("os-meter")).thenReturn(mockMeter); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry); + when(mockMeter.upDownCounterBuilder(counterName)).thenReturn(mockOTelLongUpDownCounterBuilder); + when(mockOTelLongUpDownCounterBuilder.setDescription(description)).thenReturn(mockOTelLongUpDownCounterBuilder); + when(mockOTelLongUpDownCounterBuilder.setUnit(unit)).thenReturn(mockOTelLongUpDownCounterBuilder); + when(mockOTelLongUpDownCounterBuilder.ofDoubles()).thenReturn(mockOTelDoubleUpDownCounterBuilder); + when(mockOTelDoubleUpDownCounterBuilder.build()).thenReturn(mockOTelUpDownDoubleCounter); + + Counter counter = metricsTelemetry.createUpDownCounter(counterName, description, unit); + counter.add(1.0); + verify(mockOTelUpDownDoubleCounter).add(1.0); + Attributes attributes = Attributes.create().addAttribute("test", "test"); + counter.add(-2.0, attributes); + verify(mockOTelUpDownDoubleCounter).add(-2.0, OTelAttributesConverter.convert(attributes)); + } +} diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelAttributesConverterTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelAttributesConverterTests.java index d992daec1b7bb..5d52eb0f52aab 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelAttributesConverterTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelAttributesConverterTests.java @@ -8,6 +8,7 @@ package org.opensearch.telemetry.tracing; +import org.opensearch.telemetry.OTelAttributesConverter; import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.test.OpenSearchTestCase; diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 4cd3490cffb4c..58e56575ab881 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -694,6 +694,10 @@ public void apply(Settings value, Settings current, Settings previous) { SearchService.CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_SETTING ), List.of(FeatureFlags.TELEMETRY), - List.of(TelemetrySettings.TRACER_ENABLED_SETTING, TelemetrySettings.TRACER_SAMPLER_PROBABILITY) + List.of( + TelemetrySettings.TRACER_ENABLED_SETTING, + TelemetrySettings.TRACER_SAMPLER_PROBABILITY, + TelemetrySettings.METRICS_ENABLED_SETTING + ) ); } diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 02f6bdd5ad24c..eeac38ac0b57e 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -224,6 +224,9 @@ import org.opensearch.tasks.consumer.TopNSearchTasksLogger; import org.opensearch.telemetry.TelemetryModule; import org.opensearch.telemetry.TelemetrySettings; +import org.opensearch.telemetry.metrics.MetricsRegistry; +import org.opensearch.telemetry.metrics.MetricsRegistryFactory; +import org.opensearch.telemetry.metrics.NoopMetricsRegistryFactory; import org.opensearch.telemetry.tracing.NoopTracerFactory; import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.telemetry.tracing.TracerFactory; @@ -392,6 +395,8 @@ public static class DiscoverySettings { private final LocalNodeFactory localNodeFactory; private final NodeService nodeService; private final Tracer tracer; + + private final MetricsRegistry metricsRegistry; final NamedWriteableRegistry namedWriteableRegistry; private final AtomicReference runnableTaskListener; private FileCache fileCache; @@ -591,17 +596,22 @@ protected Node( } TracerFactory tracerFactory; + MetricsRegistryFactory metricsRegistryFactory; if (FeatureFlags.isEnabled(TELEMETRY)) { final TelemetrySettings telemetrySettings = new TelemetrySettings(settings, clusterService.getClusterSettings()); List telemetryPlugins = pluginsService.filterPlugins(TelemetryPlugin.class); TelemetryModule telemetryModule = new TelemetryModule(telemetryPlugins, telemetrySettings); tracerFactory = new TracerFactory(telemetrySettings, telemetryModule.getTelemetry(), threadPool.getThreadContext()); + metricsRegistryFactory = new MetricsRegistryFactory(telemetrySettings, telemetryModule.getTelemetry()); } else { tracerFactory = new NoopTracerFactory(); + metricsRegistryFactory = new NoopMetricsRegistryFactory(); } tracer = tracerFactory.getTracer(); + metricsRegistry = metricsRegistryFactory.getMeterRegistry(); resourcesToClose.add(tracer::close); + resourcesToClose.add(metricsRegistry::close); final IngestService ingestService = new IngestService( clusterService, threadPool, @@ -1206,6 +1216,7 @@ protected Node( b.bind(IdentityService.class).toInstance(identityService); b.bind(Tracer.class).toInstance(tracer); b.bind(SearchRequestStats.class).toInstance(searchRequestStats); + b.bind(MetricsRegistry.class).toInstance(metricsRegistry); b.bind(RemoteClusterStateService.class).toProvider(() -> remoteClusterStateService); b.bind(PersistedStateRegistry.class).toInstance(persistedStateRegistry); }); @@ -1573,6 +1584,7 @@ public synchronized void close() throws IOException { toClose.add(stopWatch::stop); if (FeatureFlags.isEnabled(TELEMETRY)) { toClose.add(injector.getInstance(Tracer.class)); + toClose.add(injector.getInstance(MetricsRegistry.class)); } if (logger.isTraceEnabled()) { diff --git a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java index dc0b04244296f..71ca5fdba2395 100644 --- a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java +++ b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java @@ -27,6 +27,13 @@ public class TelemetrySettings { Setting.Property.Dynamic ); + public static final Setting METRICS_ENABLED_SETTING = Setting.boolSetting( + "telemetry.metrics.enabled", + false, + Setting.Property.NodeScope, + Setting.Property.Dynamic + ); + /** * Probability of sampler */ @@ -42,12 +49,16 @@ public class TelemetrySettings { private volatile boolean tracingEnabled; private volatile double samplingProbability; + private volatile boolean metricsEnabled; + public TelemetrySettings(Settings settings, ClusterSettings clusterSettings) { this.tracingEnabled = TRACER_ENABLED_SETTING.get(settings); this.samplingProbability = TRACER_SAMPLER_PROBABILITY.get(settings); + this.metricsEnabled = METRICS_ENABLED_SETTING.get(settings); clusterSettings.addSettingsUpdateConsumer(TRACER_ENABLED_SETTING, this::setTracingEnabled); clusterSettings.addSettingsUpdateConsumer(TRACER_SAMPLER_PROBABILITY, this::setSamplingProbability); + clusterSettings.addSettingsUpdateConsumer(METRICS_ENABLED_SETTING, this::setMetricsEnabled); } public void setTracingEnabled(boolean tracingEnabled) { @@ -72,4 +83,21 @@ public void setSamplingProbability(double samplingProbability) { public double getSamplingProbability() { return samplingProbability; } + + /** + * update the metrics enabled property. + * @param metricsEnabled metrics enabled. + */ + public void setMetricsEnabled(boolean metricsEnabled) { + this.metricsEnabled = metricsEnabled; + } + + /** + * Returns whether metrics are enabled or not. + * @return enabled/disabled flag. + */ + public boolean isMetricsEnabled() { + return metricsEnabled; + } + } diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java b/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java new file mode 100644 index 0000000000000..7d4195ebda326 --- /dev/null +++ b/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java @@ -0,0 +1,81 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.common.annotation.InternalApi; +import org.opensearch.telemetry.Telemetry; +import org.opensearch.telemetry.TelemetrySettings; +import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry; +import org.opensearch.telemetry.tracing.Tracer; + +import java.io.Closeable; +import java.io.IOException; +import java.util.Optional; + +/** + * {@link MetricsRegistryFactory} represents a single global class that is used to access meters. + *

+ * The {@link MetricsRegistry} singleton object can be retrieved using MetricsRegistryFactory::getMeterRegistry. The MeterFactroy object + * is created during class initialization and cannot subsequently be changed. + * + * @opensearch.internal + */ +@InternalApi +public class MetricsRegistryFactory implements Closeable { + + private static final Logger logger = LogManager.getLogger(MetricsRegistryFactory.class); + + private final TelemetrySettings telemetrySettings; + private final MetricsRegistry metricsRegistry; + + public MetricsRegistryFactory(TelemetrySettings telemetrySettings, Optional telemetry) { + this.telemetrySettings = telemetrySettings; + this.metricsRegistry = metricsRegistry(telemetry); + } + + /** + * Returns the meter instance + * + * @return meter instance + */ + public MetricsRegistry getMeterRegistry() { + return metricsRegistry; + } + + /** + * Closes the {@link Tracer} + */ + @Override + public void close() { + try { + metricsRegistry.close(); + } catch (IOException e) { + logger.warn("Error closing meter", e); + } + } + + private MetricsRegistry metricsRegistry(Optional telemetry) { + MetricsRegistry meter = telemetry.map(Telemetry::getMetricsTelemetry) + .map(metricsTelemetry -> createDefaultMeter(metricsTelemetry)) + .map(defaultTracer -> createWrappedMeterRegistry(defaultTracer)) + .orElse(NoopMetricsRegistry.INSTANCE); + return meter; + } + + private MetricsRegistry createDefaultMeter(MetricsTelemetry metricsTelemetry) { + return new DefaultMetricsRegistry(metricsTelemetry); + } + + private MetricsRegistry createWrappedMeterRegistry(MetricsRegistry defaultMeter) { + return new WrappedMetricsRegistry(telemetrySettings, defaultMeter); + } + +} diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/NoopMetricsRegistryFactory.java b/server/src/main/java/org/opensearch/telemetry/metrics/NoopMetricsRegistryFactory.java new file mode 100644 index 0000000000000..ad45627ea5ee2 --- /dev/null +++ b/server/src/main/java/org/opensearch/telemetry/metrics/NoopMetricsRegistryFactory.java @@ -0,0 +1,36 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics; + +import org.opensearch.common.annotation.InternalApi; +import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry; + +import java.util.Optional; + +/** + * No-op implementation of {@link MetricsRegistryFactory} + * + * @opensearch.internal + */ +@InternalApi +public class NoopMetricsRegistryFactory extends MetricsRegistryFactory { + public NoopMetricsRegistryFactory() { + super(null, Optional.empty()); + } + + @Override + public MetricsRegistry getMeterRegistry() { + return NoopMetricsRegistry.INSTANCE; + } + + @Override + public void close() { + + } +} diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java b/server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java new file mode 100644 index 0000000000000..9ffa02fb8a2ba --- /dev/null +++ b/server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java @@ -0,0 +1,51 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics; + +import org.opensearch.common.annotation.InternalApi; +import org.opensearch.telemetry.TelemetrySettings; +import org.opensearch.telemetry.tracing.attributes.Attributes; + +/** + * Wrapper implementation of {@link Counter}. This delegates call to right {@link Counter} based on the settings + */ +@InternalApi +public class WrappedCounter implements Counter { + + private final TelemetrySettings telemetrySettings; + private final Counter delegate; + + /** + * Constructor + * @param telemetrySettings telemetry settings. + * @param delegate delegate counter. + */ + public WrappedCounter(TelemetrySettings telemetrySettings, Counter delegate) { + this.telemetrySettings = telemetrySettings; + this.delegate = delegate; + } + + @Override + public void add(double value) { + if (isMetricsEnabled()) { + delegate.add(value); + } + } + + @Override + public void add(double value, Attributes attributes) { + if (isMetricsEnabled()) { + delegate.add(value, attributes); + } + } + + private boolean isMetricsEnabled() { + return telemetrySettings.isMetricsEnabled(); + } +} diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistry.java b/server/src/main/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistry.java new file mode 100644 index 0000000000000..c83ef21501d4e --- /dev/null +++ b/server/src/main/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistry.java @@ -0,0 +1,49 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics; + +import org.opensearch.common.annotation.InternalApi; +import org.opensearch.telemetry.TelemetrySettings; + +import java.io.IOException; + +/** + * Wrapper implementation of {@link MetricsRegistry}. This delegates call to right {@link MetricsRegistry} based on the settings + */ +@InternalApi +public class WrappedMetricsRegistry implements MetricsRegistry { + + private final MetricsRegistry defaultMetricsRegistry; + private final TelemetrySettings telemetrySettings; + + /** + * Constructor. + * @param defaultMetricsRegistry default meter registry. + * @param telemetrySettings telemetry settings. + */ + public WrappedMetricsRegistry(TelemetrySettings telemetrySettings, MetricsRegistry defaultMetricsRegistry) { + this.telemetrySettings = telemetrySettings; + this.defaultMetricsRegistry = defaultMetricsRegistry; + } + + @Override + public Counter createCounter(String name, String description, String unit) { + return new WrappedCounter(telemetrySettings, defaultMetricsRegistry.createCounter(name, description, unit)); + } + + @Override + public Counter createUpDownCounter(String name, String description, String unit) { + return new WrappedCounter(telemetrySettings, defaultMetricsRegistry.createUpDownCounter(name, description, unit)); + } + + @Override + public void close() throws IOException { + defaultMetricsRegistry.close(); + } +} diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/package-info.java b/server/src/main/java/org/opensearch/telemetry/metrics/package-info.java new file mode 100644 index 0000000000000..ad4564e1d7773 --- /dev/null +++ b/server/src/main/java/org/opensearch/telemetry/metrics/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * This package contains classes needed for telemetry. + */ +package org.opensearch.telemetry.metrics; diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/MetricsRegistryFactoryTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/metrics/MetricsRegistryFactoryTests.java new file mode 100644 index 0000000000000..dfde419281f7b --- /dev/null +++ b/server/src/test/java/org/opensearch/telemetry/tracing/metrics/MetricsRegistryFactoryTests.java @@ -0,0 +1,75 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing.metrics; + +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; +import org.opensearch.telemetry.Telemetry; +import org.opensearch.telemetry.TelemetrySettings; +import org.opensearch.telemetry.metrics.MetricsRegistry; +import org.opensearch.telemetry.metrics.MetricsRegistryFactory; +import org.opensearch.telemetry.metrics.MetricsTelemetry; +import org.opensearch.telemetry.metrics.WrappedMetricsRegistry; +import org.opensearch.telemetry.metrics.noop.NoopCounter; +import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry; +import org.opensearch.telemetry.tracing.TracingTelemetry; +import org.opensearch.test.OpenSearchTestCase; +import org.junit.After; + +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class MetricsRegistryFactoryTests extends OpenSearchTestCase { + + private MetricsRegistryFactory metricsRegistryFactory; + + @After + public void close() { + metricsRegistryFactory.close(); + } + + public void testGetMeterRegistrywithUnavailableMetricsTelemetry() { + Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), false).build(); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); + Telemetry mockTelemetry = mock(Telemetry.class); + when(mockTelemetry.getTracingTelemetry()).thenReturn(mock(TracingTelemetry.class)); + metricsRegistryFactory = new MetricsRegistryFactory(telemetrySettings, Optional.empty()); + + MetricsRegistry metricsRegistry = metricsRegistryFactory.getMeterRegistry(); + + assertTrue(metricsRegistry instanceof NoopMetricsRegistry); + assertTrue(metricsRegistry.createCounter("test", "test", "test") == NoopCounter.INSTANCE); + assertTrue(metricsRegistry.createUpDownCounter("test", "test", "test") == NoopCounter.INSTANCE); + } + + public void testGetTracerWithAvailableTracingTelemetryReturnsWrappedTracer() { + Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), true).build(); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); + Telemetry mockTelemetry = mock(Telemetry.class); + when(mockTelemetry.getMetricsTelemetry()).thenReturn(mock(MetricsTelemetry.class)); + metricsRegistryFactory = new MetricsRegistryFactory(telemetrySettings, Optional.of(mockTelemetry)); + + MetricsRegistry metricsRegistry = metricsRegistryFactory.getMeterRegistry(); + assertTrue(metricsRegistry instanceof WrappedMetricsRegistry); + + } + + private Set> getClusterSettings() { + Set> allTracerSettings = new HashSet<>(); + ClusterSettings.FEATURE_FLAGGED_CLUSTER_SETTINGS.get(List.of(FeatureFlags.TELEMETRY)).stream().forEach((allTracerSettings::add)); + return allTracerSettings; + } +} diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java new file mode 100644 index 0000000000000..9f589b879f41e --- /dev/null +++ b/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java @@ -0,0 +1,65 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing.metrics; + +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; +import org.opensearch.telemetry.TelemetrySettings; +import org.opensearch.telemetry.metrics.Counter; +import org.opensearch.telemetry.metrics.WrappedCounter; +import org.opensearch.telemetry.tracing.attributes.Attributes; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +public class WrappedCounterTests extends OpenSearchTestCase { + + public void testCounterWithDisabledMetrics() throws Exception { + Settings settings = Settings.builder().put(TelemetrySettings.METRICS_ENABLED_SETTING.getKey(), false).build(); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); + Counter mockCounter = mock(Counter.class); + + WrappedCounter wrappedCounter = new WrappedCounter(telemetrySettings, mockCounter); + wrappedCounter.add(1.0); + verify(mockCounter, never()).add(1.0); + + Attributes attributes = Attributes.create().addAttribute("test", "test"); + wrappedCounter.add(1.0, attributes); + verify(mockCounter, never()).add(1.0, attributes); + + } + + public void testCounterWithEnabledMetrics() throws Exception { + Settings settings = Settings.builder().put(TelemetrySettings.METRICS_ENABLED_SETTING.getKey(), true).build(); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); + Counter mockCounter = mock(Counter.class); + + WrappedCounter wrappedCounter = new WrappedCounter(telemetrySettings, mockCounter); + wrappedCounter.add(1.0); + verify(mockCounter).add(1.0); + + Attributes attributes = Attributes.create().addAttribute("test", "test"); + wrappedCounter.add(1.0, attributes); + verify(mockCounter).add(1.0, attributes); + } + + private Set> getClusterSettings() { + Set> allTracerSettings = new HashSet<>(); + ClusterSettings.FEATURE_FLAGGED_CLUSTER_SETTINGS.get(List.of(FeatureFlags.TELEMETRY)).stream().forEach((allTracerSettings::add)); + return allTracerSettings; + } +} diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedMetricsRegistryTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedMetricsRegistryTests.java new file mode 100644 index 0000000000000..9c0bceed3c8e6 --- /dev/null +++ b/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedMetricsRegistryTests.java @@ -0,0 +1,54 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.tracing.metrics; + +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; +import org.opensearch.telemetry.TelemetrySettings; +import org.opensearch.telemetry.metrics.Counter; +import org.opensearch.telemetry.metrics.DefaultMetricsRegistry; +import org.opensearch.telemetry.metrics.MetricsTelemetry; +import org.opensearch.telemetry.metrics.WrappedCounter; +import org.opensearch.telemetry.metrics.WrappedMetricsRegistry; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class WrappedMetricsRegistryTests extends OpenSearchTestCase { + + public void testCounter() throws Exception { + Settings settings = Settings.builder().put(TelemetrySettings.METRICS_ENABLED_SETTING.getKey(), false).build(); + TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); + MetricsTelemetry mockMetricsTelemetry = mock(MetricsTelemetry.class); + Counter mockCounter = mock(Counter.class); + Counter mockUpDownCounter = mock(Counter.class); + DefaultMetricsRegistry defaultMeterRegistry = new DefaultMetricsRegistry(mockMetricsTelemetry); + + when(mockMetricsTelemetry.createCounter("test", "test", "test")).thenReturn(mockCounter); + when(mockMetricsTelemetry.createUpDownCounter("test", "test", "test")).thenReturn(mockUpDownCounter); + + WrappedMetricsRegistry wrappedMeterRegistry = new WrappedMetricsRegistry(telemetrySettings, defaultMeterRegistry); + assertTrue(wrappedMeterRegistry.createCounter("test", "test", "test") instanceof WrappedCounter); + assertTrue(wrappedMeterRegistry.createUpDownCounter("test", "test", "test") instanceof WrappedCounter); + } + + private Set> getClusterSettings() { + Set> allTracerSettings = new HashSet<>(); + ClusterSettings.FEATURE_FLAGGED_CLUSTER_SETTINGS.get(List.of(FeatureFlags.TELEMETRY)).stream().forEach((allTracerSettings::add)); + return allTracerSettings; + } + +} diff --git a/test/framework/src/main/java/org/opensearch/test/telemetry/MockTelemetry.java b/test/framework/src/main/java/org/opensearch/test/telemetry/MockTelemetry.java index 6b428a7f65594..95321a7009be9 100644 --- a/test/framework/src/main/java/org/opensearch/test/telemetry/MockTelemetry.java +++ b/test/framework/src/main/java/org/opensearch/test/telemetry/MockTelemetry.java @@ -10,6 +10,7 @@ import org.opensearch.telemetry.Telemetry; import org.opensearch.telemetry.TelemetrySettings; +import org.opensearch.telemetry.metrics.Counter; import org.opensearch.telemetry.metrics.MetricsTelemetry; import org.opensearch.telemetry.tracing.TracingTelemetry; import org.opensearch.test.telemetry.tracing.MockTracingTelemetry; @@ -34,6 +35,20 @@ public TracingTelemetry getTracingTelemetry() { @Override public MetricsTelemetry getMetricsTelemetry() { return new MetricsTelemetry() { + @Override + public Counter createCounter(String name, String description, String unit) { + return null; + } + + @Override + public Counter createUpDownCounter(String name, String description, String unit) { + return null; + } + + @Override + public void close() { + + } }; } } From f02f0e72c5d3054f057a60f4175cb7009bc82474 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Tue, 26 Sep 2023 20:25:37 +0530 Subject: [PATCH 02/19] Add CHANGELOG Signed-off-by: Gagan Juneja --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aea4a45b60ffc..153cc12c5f9bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -130,6 +130,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add instrumentation in Inbound Handler. ([#100143](https://github.com/opensearch-project/OpenSearch/pull/10143)) - Enable remote segment upload backpressure by default ([#10356](https://github.com/opensearch-project/OpenSearch/pull/10356)) - [Remote Store] Add support to reload repository metadata inplace ([#9569](https://github.com/opensearch-project/OpenSearch/pull/9569)) +- [Metrics Framework] Add Metrics framework. ([#10241](https://github.com/opensearch-project/OpenSearch/pull/10241)) ### Deprecated @@ -147,4 +148,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Security [Unreleased 3.0]: https://github.com/opensearch-project/OpenSearch/compare/2.x...HEAD -[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.11...2.x \ No newline at end of file +[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.11...2.x From 8337f58ff046f80d8f164af9dcb3d413011af9d5 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Tue, 26 Sep 2023 21:06:24 +0530 Subject: [PATCH 03/19] Fix test case Signed-off-by: Gagan Juneja --- .../org/opensearch/telemetry/OTelTelemetryPluginTests.java | 4 ++-- .../telemetry/metrics/OTelMetricsTelemetryTests.java | 4 ++-- .../telemetry/tracing/sampler/ProbabilisticSamplerTests.java | 5 +++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java index 46100414e86c8..5730a90ddb0f0 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java @@ -13,7 +13,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; import org.opensearch.telemetry.metrics.MetricsTelemetry; -import org.opensearch.telemetry.metrics.OTelMetricsTelemetryTests; +import org.opensearch.telemetry.metrics.OTelMetricsTelemetry; import org.opensearch.telemetry.tracing.OTelTracingTelemetry; import org.opensearch.telemetry.tracing.TracingTelemetry; import org.opensearch.test.OpenSearchTestCase; @@ -65,7 +65,7 @@ public void testGetTelemetry() { ClusterSettings.FEATURE_FLAGGED_CLUSTER_SETTINGS.get(List.of(FeatureFlags.TELEMETRY)).stream().forEach((allTracerSettings::add)); assertEquals(OTEL_TRACER_NAME, oTelTelemetryModulePlugin.getName()); assertTrue(tracingTelemetry instanceof OTelTracingTelemetry); - assertTrue(metricsTelemetry instanceof OTelMetricsTelemetryTests); + assertTrue(metricsTelemetry instanceof OTelMetricsTelemetry); assertEquals( Arrays.asList( TRACER_EXPORTER_BATCH_SIZE_SETTING, diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java index 3bd306f46dbce..db8e516350395 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java @@ -73,7 +73,7 @@ public void testCounterNegativeValue() { Counter counter = metricsTelemetry.createCounter(counterName, description, unit); counter.add(-1.0); - verify(mockOTelDoubleCounter).add(1.0); + verify(mockOTelDoubleCounter).add(-1.0); } public void testUpDownCounter() { @@ -99,6 +99,6 @@ public void testUpDownCounter() { verify(mockOTelUpDownDoubleCounter).add(1.0); Attributes attributes = Attributes.create().addAttribute("test", "test"); counter.add(-2.0, attributes); - verify(mockOTelUpDownDoubleCounter).add(-2.0, OTelAttributesConverter.convert(attributes)); + verify(mockOTelUpDownDoubleCounter).add((-2.0), OTelAttributesConverter.convert(attributes)); } } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSamplerTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSamplerTests.java index 639dc341ef0db..5cf7cf127822c 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSamplerTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSamplerTests.java @@ -18,6 +18,7 @@ import io.opentelemetry.sdk.trace.samplers.Sampler; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING; +import static org.opensearch.telemetry.TelemetrySettings.METRICS_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_PROBABILITY; @@ -33,7 +34,7 @@ public void testDefaultGetSampler() { Settings settings = Settings.builder().put(TRACER_EXPORTER_DELAY_SETTING.getKey(), "1s").build(); TelemetrySettings telemetrySettings = new TelemetrySettings( Settings.EMPTY, - new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)) + new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING, METRICS_ENABLED_SETTING)) ); // Probabilistic Sampler @@ -47,7 +48,7 @@ public void testGetSamplerWithUpdatedSamplingRatio() { Settings settings = Settings.builder().put(TRACER_EXPORTER_DELAY_SETTING.getKey(), "1s").build(); TelemetrySettings telemetrySettings = new TelemetrySettings( Settings.EMPTY, - new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)) + new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING, METRICS_ENABLED_SETTING)) ); // Probabilistic Sampler From 9bad4eb371de62c0aa1f34dfba1118989bd285e6 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Sun, 1 Oct 2023 18:22:55 +0530 Subject: [PATCH 04/19] Address review comment Signed-off-by: Gagan Juneja --- .../config/telemetry-otel/log4j2.properties | 20 ++++ .../telemetry/OTelTelemetryPlugin.java | 3 +- .../telemetry/OTelTelemetrySettings.java | 27 +++++ .../exporter/OTelMetricsExporterFactory.java | 99 +++++++++++++++++++ .../metrics/exporter/package-info.java | 12 +++ .../telemetry/OTelTelemetryPluginTests.java | 4 +- .../main/java/org/opensearch/node/Node.java | 2 +- .../metrics/MetricsRegistryFactory.java | 26 ++--- .../metrics/NoopMetricsRegistryFactory.java | 2 +- .../metrics/MetricsRegistryFactoryTests.java | 4 +- 10 files changed, 180 insertions(+), 19 deletions(-) create mode 100644 plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/exporter/OTelMetricsExporterFactory.java create mode 100644 plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/exporter/package-info.java diff --git a/plugins/telemetry-otel/config/telemetry-otel/log4j2.properties b/plugins/telemetry-otel/config/telemetry-otel/log4j2.properties index 544f42bd5513b..8dec1119eec66 100644 --- a/plugins/telemetry-otel/config/telemetry-otel/log4j2.properties +++ b/plugins/telemetry-otel/config/telemetry-otel/log4j2.properties @@ -25,3 +25,23 @@ logger.exporter.name = io.opentelemetry.exporter.logging.LoggingSpanExporter logger.exporter.level = INFO logger.exporter.appenderRef.tracing.ref = tracing logger.exporter.additivity = false + + +appender.metrics.type = RollingFile +appender.metrics.name = metrics +appender.metrics.fileName = ${sys:opensearch.logs.base_path}${sys:file.separator}${sys:opensearch.logs.cluster_name}_otel_metrics.log +appender.metrics.filePermissions = rw-r----- +appender.metrics.layout.type = PatternLayout +appender.metrics.layout.pattern = %m%n +appender.metrics.filePattern = ${sys:opensearch.logs.base_path}${sys:file.separator}${sys:opensearch.logs.cluster_name}_otel_metrics-%i.log.gz +appender.metrics.policies.type = Policies +appender.metrics.policies.size.type = SizeBasedTriggeringPolicy +appender.metrics.policies.size.size = 1GB +appender.metrics.strategy.type = DefaultRolloverStrategy +appender.metrics.strategy.max = 4 + + +logger.metrics_exporter.name = io.opentelemetry.exporter.logging.LoggingMetricExporter +logger.metrics_exporter.level = INFO +logger.metrics_exporter.appenderRef.tracing.ref = metrics +logger.metrics_exporter.additivity = false diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java index 97ed7303643cb..8ceecbce9e5da 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java @@ -47,7 +47,8 @@ public List> getSettings() { OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING, OTelTelemetrySettings.TRACER_EXPORTER_MAX_QUEUE_SIZE_SETTING, OTelTelemetrySettings.OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING, - OTelTelemetrySettings.METRICS_PUBLISH_INTERVAL_SETTING + OTelTelemetrySettings.METRICS_PUBLISH_INTERVAL_SETTING, + OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING ); } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java index 4d1174cddc8b2..c7f84e3725a47 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java @@ -11,13 +11,16 @@ import org.opensearch.SpecialPermission; import org.opensearch.common.settings.Setting; import org.opensearch.common.unit.TimeValue; +import org.opensearch.telemetry.metrics.exporter.OTelMetricsExporterFactory; import org.opensearch.telemetry.tracing.exporter.OTelSpanExporterFactory; import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; +import io.opentelemetry.exporter.logging.LoggingMetricExporter; import io.opentelemetry.exporter.logging.LoggingSpanExporter; +import io.opentelemetry.sdk.metrics.export.MetricExporter; import io.opentelemetry.sdk.trace.export.SpanExporter; /** @@ -93,4 +96,28 @@ private OTelTelemetrySettings() {} Setting.Property.NodeScope, Setting.Property.Final ); + + /** + * Metrics Exporter type setting. + */ + @SuppressWarnings("unchecked") + public static final Setting> OTEL_METRICS_EXPORTER_CLASS_SETTING = new Setting<>( + "telemetry.otel.metrics.exporter.class", + LoggingMetricExporter.class.getName(), + className -> { + // Check we ourselves are not being called by unprivileged code. + SpecialPermission.check(); + + try { + return AccessController.doPrivileged((PrivilegedExceptionAction>) () -> { + final ClassLoader loader = OTelMetricsExporterFactory.class.getClassLoader(); + return (Class) loader.loadClass(className); + }); + } catch (PrivilegedActionException ex) { + throw new IllegalStateException("Unable to load span exporter class:" + className, ex.getCause()); + } + }, + Setting.Property.NodeScope, + Setting.Property.Final + ); } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/exporter/OTelMetricsExporterFactory.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/exporter/OTelMetricsExporterFactory.java new file mode 100644 index 0000000000000..975d86f2f8db6 --- /dev/null +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/exporter/OTelMetricsExporterFactory.java @@ -0,0 +1,99 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics.exporter; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.SpecialPermission; +import org.opensearch.common.settings.Settings; +import org.opensearch.telemetry.OTelTelemetrySettings; + +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.lang.reflect.Method; +import java.security.AccessController; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; + +import io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter; +import io.opentelemetry.sdk.metrics.export.AggregationTemporalitySelector; +import io.opentelemetry.sdk.metrics.export.MetricExporter; + +/** + * Factory class to create the {@link MetricExporter} instance. + */ +public class OTelMetricsExporterFactory { + + private static final Logger logger = LogManager.getLogger(OTelMetricsExporterFactory.class); + + /** + * Base constructor. + */ + private OTelMetricsExporterFactory() { + + } + + /** + * Creates the {@link MetricExporter} instances based on the OTEL_METRIC_EXPORTER_CLASS_SETTING value. + * As of now, it expects the MetricExporter implementations to have a create factory method to instantiate the + * MetricExporter. + * @param settings settings. + * @return MetricExporter instance. + */ + public static MetricExporter create(Settings settings) { + Class MetricExporterProviderClass = OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING.get(settings); + MetricExporter metricExporter; + if (MetricExporterProviderClass.getName().equals("io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter")) { + metricExporter = OtlpGrpcMetricExporter.builder() + .setAggregationTemporalitySelector(AggregationTemporalitySelector.deltaPreferred()) + .build(); + } else { + metricExporter = instantiateExporter(MetricExporterProviderClass); + } + logger.info("Successfully instantiated the Metrics Exporter class {}", MetricExporterProviderClass); + return metricExporter; + } + + private static MetricExporter instantiateExporter(Class exporterProviderClass) { + try { + // Check we ourselves are not being called by unprivileged code. + SpecialPermission.check(); + return AccessController.doPrivileged((PrivilegedExceptionAction) () -> { + String methodName = "create"; + String getDefaultMethod = "getDefault"; + for (Method m : exporterProviderClass.getMethods()) { + if (m.getName().equals(getDefaultMethod)) { + methodName = getDefaultMethod; + break; + } + } + try { + return (MetricExporter) MethodHandles.publicLookup() + .findStatic(exporterProviderClass, methodName, MethodType.methodType(exporterProviderClass)) + .asType(MethodType.methodType(MetricExporter.class)) + .invokeExact(); + } catch (Throwable e) { + if (e.getCause() instanceof NoSuchMethodException) { + throw new IllegalStateException("No create factory method exist in [" + exporterProviderClass.getName() + "]"); + } else { + throw new IllegalStateException( + "Exporter instantiation failed for class [" + exporterProviderClass.getName() + "]", + e.getCause() + ); + } + } + }); + } catch (PrivilegedActionException ex) { + throw new IllegalStateException( + "Exporter instantiation failed for class [" + exporterProviderClass.getName() + "]", + ex.getCause() + ); + } + } +} diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/exporter/package-info.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/exporter/package-info.java new file mode 100644 index 0000000000000..b48ec3e2336c4 --- /dev/null +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/exporter/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * This package contains classes needed for tracing requests. + */ +package org.opensearch.telemetry.metrics.exporter; diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java index 5730a90ddb0f0..4de5b4d683dc2 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java @@ -28,6 +28,7 @@ import static org.opensearch.telemetry.OTelTelemetryPlugin.OTEL_TRACER_NAME; import static org.opensearch.telemetry.OTelTelemetrySettings.METRICS_PUBLISH_INTERVAL_SETTING; +import static org.opensearch.telemetry.OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_BATCH_SIZE_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING; @@ -72,7 +73,8 @@ public void testGetTelemetry() { TRACER_EXPORTER_DELAY_SETTING, TRACER_EXPORTER_MAX_QUEUE_SIZE_SETTING, OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING, - METRICS_PUBLISH_INTERVAL_SETTING + METRICS_PUBLISH_INTERVAL_SETTING, + OTEL_METRICS_EXPORTER_CLASS_SETTING ), oTelTelemetryModulePlugin.getSettings() ); diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index eeac38ac0b57e..f44c8c8bea7f1 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -609,7 +609,7 @@ protected Node( } tracer = tracerFactory.getTracer(); - metricsRegistry = metricsRegistryFactory.getMeterRegistry(); + metricsRegistry = metricsRegistryFactory.getMetricsRegistry(); resourcesToClose.add(tracer::close); resourcesToClose.add(metricsRegistry::close); final IngestService ingestService = new IngestService( diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java b/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java index 7d4195ebda326..9417b90500e88 100644 --- a/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java +++ b/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java @@ -21,9 +21,9 @@ import java.util.Optional; /** - * {@link MetricsRegistryFactory} represents a single global class that is used to access meters. + * {@link MetricsRegistryFactory} represents a single global class that is used to access {@link MetricsRegistry}s. *

- * The {@link MetricsRegistry} singleton object can be retrieved using MetricsRegistryFactory::getMeterRegistry. The MeterFactroy object + * The {@link MetricsRegistry} singleton object can be retrieved using MetricsRegistryFactory::getMetricsRegistry. The {@link MetricsRegistryFactory} object * is created during class initialization and cannot subsequently be changed. * * @opensearch.internal @@ -42,11 +42,11 @@ public MetricsRegistryFactory(TelemetrySettings telemetrySettings, Optional telemetry) { - MetricsRegistry meter = telemetry.map(Telemetry::getMetricsTelemetry) - .map(metricsTelemetry -> createDefaultMeter(metricsTelemetry)) - .map(defaultTracer -> createWrappedMeterRegistry(defaultTracer)) + MetricsRegistry metricsRegistry = telemetry.map(Telemetry::getMetricsTelemetry) + .map(metricsTelemetry -> createDefaultMetricsRegistry(metricsTelemetry)) + .map(defaultTracer -> createWrappedMetricsRegistry(defaultTracer)) .orElse(NoopMetricsRegistry.INSTANCE); - return meter; + return metricsRegistry; } - private MetricsRegistry createDefaultMeter(MetricsTelemetry metricsTelemetry) { + private MetricsRegistry createDefaultMetricsRegistry(MetricsTelemetry metricsTelemetry) { return new DefaultMetricsRegistry(metricsTelemetry); } - private MetricsRegistry createWrappedMeterRegistry(MetricsRegistry defaultMeter) { - return new WrappedMetricsRegistry(telemetrySettings, defaultMeter); + private MetricsRegistry createWrappedMetricsRegistry(MetricsRegistry metricsRegistry) { + return new WrappedMetricsRegistry(telemetrySettings, metricsRegistry); } } diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/NoopMetricsRegistryFactory.java b/server/src/main/java/org/opensearch/telemetry/metrics/NoopMetricsRegistryFactory.java index ad45627ea5ee2..5137cb18e2cc0 100644 --- a/server/src/main/java/org/opensearch/telemetry/metrics/NoopMetricsRegistryFactory.java +++ b/server/src/main/java/org/opensearch/telemetry/metrics/NoopMetricsRegistryFactory.java @@ -25,7 +25,7 @@ public NoopMetricsRegistryFactory() { } @Override - public MetricsRegistry getMeterRegistry() { + public MetricsRegistry getMetricsRegistry() { return NoopMetricsRegistry.INSTANCE; } diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/MetricsRegistryFactoryTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/metrics/MetricsRegistryFactoryTests.java index dfde419281f7b..656e94406b022 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/MetricsRegistryFactoryTests.java +++ b/server/src/test/java/org/opensearch/telemetry/tracing/metrics/MetricsRegistryFactoryTests.java @@ -48,7 +48,7 @@ public void testGetMeterRegistrywithUnavailableMetricsTelemetry() { when(mockTelemetry.getTracingTelemetry()).thenReturn(mock(TracingTelemetry.class)); metricsRegistryFactory = new MetricsRegistryFactory(telemetrySettings, Optional.empty()); - MetricsRegistry metricsRegistry = metricsRegistryFactory.getMeterRegistry(); + MetricsRegistry metricsRegistry = metricsRegistryFactory.getMetricsRegistry(); assertTrue(metricsRegistry instanceof NoopMetricsRegistry); assertTrue(metricsRegistry.createCounter("test", "test", "test") == NoopCounter.INSTANCE); @@ -62,7 +62,7 @@ public void testGetTracerWithAvailableTracingTelemetryReturnsWrappedTracer() { when(mockTelemetry.getMetricsTelemetry()).thenReturn(mock(MetricsTelemetry.class)); metricsRegistryFactory = new MetricsRegistryFactory(telemetrySettings, Optional.of(mockTelemetry)); - MetricsRegistry metricsRegistry = metricsRegistryFactory.getMeterRegistry(); + MetricsRegistry metricsRegistry = metricsRegistryFactory.getMetricsRegistry(); assertTrue(metricsRegistry instanceof WrappedMetricsRegistry); } From c2fd9620e79b5063737afe87c18f8b8352f3dbe0 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Mon, 2 Oct 2023 13:33:08 +0530 Subject: [PATCH 05/19] Add unit tests Signed-off-by: Gagan Juneja --- .../exporter/OTelMetricsExporterFactory.java | 17 +--- .../metrics/exporter/DummyMetricExporter.java | 39 ++++++++++ .../OTelMetricsExporterFactoryTests.java | 78 +++++++++++++++++++ 3 files changed, 121 insertions(+), 13 deletions(-) create mode 100644 plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/exporter/DummyMetricExporter.java create mode 100644 plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/exporter/OTelMetricsExporterFactoryTests.java diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/exporter/OTelMetricsExporterFactory.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/exporter/OTelMetricsExporterFactory.java index 975d86f2f8db6..ef5a31e4003ca 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/exporter/OTelMetricsExporterFactory.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/exporter/OTelMetricsExporterFactory.java @@ -21,8 +21,6 @@ import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; -import io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter; -import io.opentelemetry.sdk.metrics.export.AggregationTemporalitySelector; import io.opentelemetry.sdk.metrics.export.MetricExporter; /** @@ -48,15 +46,8 @@ private OTelMetricsExporterFactory() { */ public static MetricExporter create(Settings settings) { Class MetricExporterProviderClass = OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING.get(settings); - MetricExporter metricExporter; - if (MetricExporterProviderClass.getName().equals("io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter")) { - metricExporter = OtlpGrpcMetricExporter.builder() - .setAggregationTemporalitySelector(AggregationTemporalitySelector.deltaPreferred()) - .build(); - } else { - metricExporter = instantiateExporter(MetricExporterProviderClass); - } - logger.info("Successfully instantiated the Metrics Exporter class {}", MetricExporterProviderClass); + MetricExporter metricExporter = instantiateExporter(MetricExporterProviderClass); + logger.info("Successfully instantiated the Metrics MetricExporter class {}", MetricExporterProviderClass); return metricExporter; } @@ -83,7 +74,7 @@ private static MetricExporter instantiateExporter(Class exporter throw new IllegalStateException("No create factory method exist in [" + exporterProviderClass.getName() + "]"); } else { throw new IllegalStateException( - "Exporter instantiation failed for class [" + exporterProviderClass.getName() + "]", + "MetricExporter instantiation failed for class [" + exporterProviderClass.getName() + "]", e.getCause() ); } @@ -91,7 +82,7 @@ private static MetricExporter instantiateExporter(Class exporter }); } catch (PrivilegedActionException ex) { throw new IllegalStateException( - "Exporter instantiation failed for class [" + exporterProviderClass.getName() + "]", + "MetricExporter instantiation failed for class [" + exporterProviderClass.getName() + "]", ex.getCause() ); } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/exporter/DummyMetricExporter.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/exporter/DummyMetricExporter.java new file mode 100644 index 0000000000000..65c52911dbef9 --- /dev/null +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/exporter/DummyMetricExporter.java @@ -0,0 +1,39 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics.exporter; + +import java.util.Collection; + +import io.opentelemetry.sdk.common.CompletableResultCode; +import io.opentelemetry.sdk.metrics.InstrumentType; +import io.opentelemetry.sdk.metrics.data.AggregationTemporality; +import io.opentelemetry.sdk.metrics.data.MetricData; +import io.opentelemetry.sdk.metrics.export.MetricExporter; + +public class DummyMetricExporter implements MetricExporter { + @Override + public CompletableResultCode export(Collection metrics) { + return null; + } + + @Override + public CompletableResultCode flush() { + return null; + } + + @Override + public CompletableResultCode shutdown() { + return null; + } + + @Override + public AggregationTemporality getAggregationTemporality(InstrumentType instrumentType) { + return null; + } +} diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/exporter/OTelMetricsExporterFactoryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/exporter/OTelMetricsExporterFactoryTests.java new file mode 100644 index 0000000000000..e68da030bfb52 --- /dev/null +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/exporter/OTelMetricsExporterFactoryTests.java @@ -0,0 +1,78 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics.exporter; + +import org.opensearch.common.settings.Settings; +import org.opensearch.telemetry.OTelTelemetrySettings; +import org.opensearch.test.OpenSearchTestCase; + +import io.opentelemetry.exporter.logging.LoggingMetricExporter; +import io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter; +import io.opentelemetry.sdk.metrics.export.MetricExporter; + +public class OTelMetricsExporterFactoryTests extends OpenSearchTestCase { + + public void testMetricsExporterDefault() { + Settings settings = Settings.builder().build(); + MetricExporter metricExporter = OTelMetricsExporterFactory.create(settings); + assertTrue(metricExporter instanceof LoggingMetricExporter); + } + + public void testMetricsExporterLogging() { + Settings settings = Settings.builder() + .put( + OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING.getKey(), + "io.opentelemetry.exporter.logging.LoggingMetricExporter" + ) + .build(); + MetricExporter metricExporter = OTelMetricsExporterFactory.create(settings); + assertTrue(metricExporter instanceof LoggingMetricExporter); + } + + public void testMetricExporterInvalid() { + Settings settings = Settings.builder().put(OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING.getKey(), "abc").build(); + assertThrows(IllegalArgumentException.class, () -> OTelMetricsExporterFactory.create(settings)); + } + + public void testMetricExporterNoCreateFactoryMethod() { + Settings settings = Settings.builder() + .put( + OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING.getKey(), + "org.opensearch.telemetry.metrics.exporter.DummyMetricExporter" + ) + .build(); + IllegalStateException exception = assertThrows(IllegalStateException.class, () -> OTelMetricsExporterFactory.create(settings)); + assertEquals( + "MetricExporter instantiation failed for class [org.opensearch.telemetry.metrics.exporter.DummyMetricExporter]", + exception.getMessage() + ); + } + + public void testMetricExporterNonMetricExporterClass() { + Settings settings = Settings.builder() + .put(OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING.getKey(), "java.lang.String") + .build(); + IllegalStateException exception = assertThrows(IllegalStateException.class, () -> OTelMetricsExporterFactory.create(settings)); + assertEquals("MetricExporter instantiation failed for class [java.lang.String]", exception.getMessage()); + assertTrue(exception.getCause() instanceof NoSuchMethodError); + + } + + public void testMetricExporterGetDefaultMethod() { + Settings settings = Settings.builder() + .put( + OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING.getKey(), + "io.opentelemetry.exporter.otlp.metrics.OtlpGrpcMetricExporter" + ) + .build(); + + assertTrue(OTelMetricsExporterFactory.create(settings) instanceof OtlpGrpcMetricExporter); + } + +} From c70b6e33c489a9c66f82e71616d05ac4af7b5eaf Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Tue, 3 Oct 2023 00:11:09 +0530 Subject: [PATCH 06/19] Address review comment Signed-off-by: Gagan Juneja --- .../opensearch/telemetry/metrics/Counter.java | 7 +- .../metrics/DefaultMetricsRegistry.java | 5 +- .../telemetry/metrics/MetricsRegistry.java | 4 + .../telemetry/metrics/MetricsTelemetry.java | 21 +--- .../telemetry/metrics/noop/NoopCounter.java | 4 +- .../telemetry/metrics/tags/Tags.java | 99 +++++++++++++++++++ .../telemetry/metrics/tags/package-info.java | 12 +++ .../telemetry/OTelAttributesConverter.java | 15 +++ .../telemetry/OTelTelemetryPlugin.java | 1 - .../telemetry/OTelTelemetrySettings.java | 10 -- .../telemetry/metrics/OTelCounter.java | 6 +- .../telemetry/metrics/OTelUpDownCounter.java | 6 +- .../tracing/OTelResourceProvider.java | 8 +- .../telemetry/OTelTelemetryPluginTests.java | 2 - .../tracing/OTelAttributesConverterTests.java | 13 ++- .../telemetry/TelemetrySettings.java | 11 +++ .../metrics/MetricsRegistryFactory.java | 2 +- .../telemetry/metrics/WrappedCounter.java | 6 +- .../tracing/metrics/WrappedCounterTests.java | 13 +-- 19 files changed, 181 insertions(+), 64 deletions(-) create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/tags/Tags.java create mode 100644 libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/tags/package-info.java diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Counter.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Counter.java index 3c77087a939a1..9acfd31d0cebf 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Counter.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Counter.java @@ -8,7 +8,7 @@ package org.opensearch.telemetry.metrics; -import org.opensearch.telemetry.tracing.attributes.Attributes; +import org.opensearch.telemetry.metrics.tags.Tags; /** * Counter adds the value to the existing metric. @@ -23,9 +23,10 @@ public interface Counter { /** * add value along with the attributes. + * * @param value value to be added. - * @param attributes attributes/dimensions of the metric. + * @param tags attributes/dimensions of the metric. */ - void add(double value, Attributes attributes); + void add(double value, Tags tags); } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistry.java index 4b3b0bd681c47..d57def9406b17 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistry.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/DefaultMetricsRegistry.java @@ -8,13 +8,12 @@ package org.opensearch.telemetry.metrics; -import java.io.Closeable; import java.io.IOException; /** * Default implementation for {@link MetricsRegistry} */ -public class DefaultMetricsRegistry implements MetricsRegistry { +class DefaultMetricsRegistry implements MetricsRegistry { private final MetricsTelemetry metricsTelemetry; /** @@ -37,6 +36,6 @@ public Counter createUpDownCounter(String name, String description, String unit) @Override public void close() throws IOException { - ((Closeable) metricsTelemetry).close(); + metricsTelemetry.close(); } } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistry.java index b5f26ae822060..61b3df089928b 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistry.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistry.java @@ -8,11 +8,15 @@ package org.opensearch.telemetry.metrics; +import org.opensearch.common.annotation.ExperimentalApi; + import java.io.Closeable; /** * MetricsRegistry helps in creating the metric instruments. + * @opensearch.experimental */ +@ExperimentalApi public interface MetricsRegistry extends Closeable { /** diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java index d0c999e7c5c52..bd7f0d5010b0b 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java @@ -18,23 +18,4 @@ * @opensearch.experimental */ @ExperimentalApi -public interface MetricsTelemetry extends Closeable { - - /** - * Creates the counter. - * @param name name of the counter. - * @param description any description about the metric. - * @param unit unit of the metric. - * @return counter - */ - Counter createCounter(String name, String description, String unit); - - /** - * Creates the upDown counter. - * @param name name of the upDown counter. - * @param description any description about the metric. - * @param unit unit of the metric. - * @return upDownCounter. - */ - Counter createUpDownCounter(String name, String description, String unit); -} +public interface MetricsTelemetry extends MetricsRegistry, Closeable {} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopCounter.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopCounter.java index fc1529157b4a7..de121de3ed465 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopCounter.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopCounter.java @@ -9,7 +9,7 @@ package org.opensearch.telemetry.metrics.noop; import org.opensearch.telemetry.metrics.Counter; -import org.opensearch.telemetry.tracing.attributes.Attributes; +import org.opensearch.telemetry.metrics.tags.Tags; /** * No-op {@link Counter} @@ -29,7 +29,7 @@ public void add(double value) { } @Override - public void add(double value, Attributes attributes) { + public void add(double value, Tags tags) { } } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/tags/Tags.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/tags/Tags.java new file mode 100644 index 0000000000000..f2a8764f8021d --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/tags/Tags.java @@ -0,0 +1,99 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.telemetry.metrics.tags; + +import org.opensearch.common.annotation.ExperimentalApi; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +/** + * Class to create tags for a meter. + * + * @opensearch.experimental + */ +@ExperimentalApi +public class Tags { + private final Map tagsMap; + /** + * Empty value. + */ + public final static Tags EMPTY = new Tags(Collections.emptyMap()); + + /** + * Factory method. + * @return tags. + */ + public static Tags create() { + return new Tags(new HashMap<>()); + } + + /** + * Constructor. + */ + private Tags(Map tagsMap) { + this.tagsMap = tagsMap; + } + + /** + * Add String attribute. + * @param key key + * @param value value + * @return Same instance. + */ + public Tags addTag(String key, String value) { + Objects.requireNonNull(value, "value cannot be null"); + tagsMap.put(key, value); + return this; + } + + /** + * Add long attribute. + * @param key key + * @param value value + * @return Same instance. + */ + public Tags addTag(String key, long value) { + tagsMap.put(key, value); + return this; + }; + + /** + * Add double attribute. + * @param key key + * @param value value + * @return Same instance. + */ + public Tags addTag(String key, double value) { + tagsMap.put(key, value); + return this; + }; + + /** + * Add boolean attribute. + * @param key key + * @param value value + * @return Same instance. + */ + public Tags addTag(String key, boolean value) { + tagsMap.put(key, value); + return this; + }; + + /** + * Returns the attribute map. + * @return tags map + */ + public Map getTagsMap() { + return Collections.unmodifiableMap(tagsMap); + } + +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/tags/package-info.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/tags/package-info.java new file mode 100644 index 0000000000000..cc184e9a80f40 --- /dev/null +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/tags/package-info.java @@ -0,0 +1,12 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** + * Contains metrics related classes + */ +package org.opensearch.telemetry.metrics.tags; diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelAttributesConverter.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelAttributesConverter.java index b6c8dfda9a27b..98d265e92ba3c 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelAttributesConverter.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelAttributesConverter.java @@ -8,6 +8,8 @@ package org.opensearch.telemetry; +import org.opensearch.telemetry.metrics.tags.Tags; + import java.util.Locale; import io.opentelemetry.api.common.Attributes; @@ -49,4 +51,17 @@ private static void addSpanAttribute(String key, Object value, AttributesBuilder throw new IllegalArgumentException(String.format(Locale.ROOT, "Span attribute value %s type not supported", value)); } } + + /** + * Attribute converter. + * @param tags attributes + * @return otel attributes. + */ + public static Attributes convert(Tags tags) { + AttributesBuilder attributesBuilder = Attributes.builder(); + if (tags != null) { + tags.getTagsMap().forEach((x, y) -> addSpanAttribute(x, y, attributesBuilder)); + } + return attributesBuilder.build(); + } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java index 8ceecbce9e5da..9692f59a413a3 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java @@ -47,7 +47,6 @@ public List> getSettings() { OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING, OTelTelemetrySettings.TRACER_EXPORTER_MAX_QUEUE_SIZE_SETTING, OTelTelemetrySettings.OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING, - OTelTelemetrySettings.METRICS_PUBLISH_INTERVAL_SETTING, OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING ); } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java index c7f84e3725a47..8e23f724b4570 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetrySettings.java @@ -63,16 +63,6 @@ private OTelTelemetrySettings() {} Setting.Property.Final ); - /** - * metrics publish interval in seconds. - */ - public static final Setting METRICS_PUBLISH_INTERVAL_SETTING = Setting.timeSetting( - "telemetry.otel.metrics.publish.interval", - TimeValue.timeValueSeconds(60), - Setting.Property.NodeScope, - Setting.Property.Final - ); - /** * Span Exporter type setting. */ diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelCounter.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelCounter.java index f50a65d8417ad..d42c6720afea6 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelCounter.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelCounter.java @@ -9,7 +9,7 @@ package org.opensearch.telemetry.metrics; import org.opensearch.telemetry.OTelAttributesConverter; -import org.opensearch.telemetry.tracing.attributes.Attributes; +import org.opensearch.telemetry.metrics.tags.Tags; import io.opentelemetry.api.metrics.DoubleCounter; @@ -34,7 +34,7 @@ public void add(double value) { } @Override - public void add(double value, Attributes attributes) { - otelDoubleCounter.add(value, OTelAttributesConverter.convert(attributes)); + public void add(double value, Tags tags) { + otelDoubleCounter.add(value, OTelAttributesConverter.convert(tags)); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelUpDownCounter.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelUpDownCounter.java index 44b24e5dbc808..2f40881996f7e 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelUpDownCounter.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelUpDownCounter.java @@ -9,7 +9,7 @@ package org.opensearch.telemetry.metrics; import org.opensearch.telemetry.OTelAttributesConverter; -import org.opensearch.telemetry.tracing.attributes.Attributes; +import org.opensearch.telemetry.metrics.tags.Tags; import io.opentelemetry.api.metrics.DoubleUpDownCounter; @@ -34,7 +34,7 @@ public void add(double value) { } @Override - public void add(double value, Attributes attributes) { - doubleUpDownCounter.add(value, OTelAttributesConverter.convert(attributes)); + public void add(double value, Tags tags) { + doubleUpDownCounter.add(value, OTelAttributesConverter.convert(tags)); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java index 0ce7de772fe7d..d436264bd0557 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java @@ -10,6 +10,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.telemetry.TelemetrySettings; +import org.opensearch.telemetry.metrics.exporter.OTelMetricsExporterFactory; import org.opensearch.telemetry.tracing.exporter.OTelSpanExporterFactory; import org.opensearch.telemetry.tracing.sampler.ProbabilisticSampler; import org.opensearch.telemetry.tracing.sampler.RequestSampler; @@ -22,10 +23,8 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; import io.opentelemetry.context.propagation.ContextPropagators; -import io.opentelemetry.exporter.logging.LoggingMetricExporter; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.metrics.SdkMeterProvider; -import io.opentelemetry.sdk.metrics.data.AggregationTemporality; import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.SdkTracerProvider; @@ -34,7 +33,6 @@ import io.opentelemetry.sdk.trace.samplers.Sampler; import io.opentelemetry.semconv.resource.attributes.ResourceAttributes; -import static org.opensearch.telemetry.OTelTelemetrySettings.METRICS_PUBLISH_INTERVAL_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_BATCH_SIZE_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_MAX_QUEUE_SIZE_SETTING; @@ -85,8 +83,8 @@ private static SdkMeterProvider createSdkMetricProvider(Settings settings, Resou return SdkMeterProvider.builder() .setResource(resource) .registerMetricReader( - PeriodicMetricReader.builder(LoggingMetricExporter.create(AggregationTemporality.DELTA)) - .setInterval(METRICS_PUBLISH_INTERVAL_SETTING.get(settings).getSeconds(), TimeUnit.SECONDS) + PeriodicMetricReader.builder(OTelMetricsExporterFactory.create(settings)) + .setInterval(TelemetrySettings.METRICS_PUBLISH_INTERVAL_SETTING.get(settings).getSeconds(), TimeUnit.SECONDS) .build() ) .build(); diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java index 4de5b4d683dc2..795cdeb2ebc56 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java @@ -27,7 +27,6 @@ import java.util.Set; import static org.opensearch.telemetry.OTelTelemetryPlugin.OTEL_TRACER_NAME; -import static org.opensearch.telemetry.OTelTelemetrySettings.METRICS_PUBLISH_INTERVAL_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.OTEL_METRICS_EXPORTER_CLASS_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_BATCH_SIZE_SETTING; @@ -73,7 +72,6 @@ public void testGetTelemetry() { TRACER_EXPORTER_DELAY_SETTING, TRACER_EXPORTER_MAX_QUEUE_SIZE_SETTING, OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING, - METRICS_PUBLISH_INTERVAL_SETTING, OTEL_METRICS_EXPORTER_CLASS_SETTING ), oTelTelemetryModulePlugin.getSettings() diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelAttributesConverterTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelAttributesConverterTests.java index 5d52eb0f52aab..ee67384d01759 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelAttributesConverterTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelAttributesConverterTests.java @@ -9,6 +9,7 @@ package org.opensearch.telemetry.tracing; import org.opensearch.telemetry.OTelAttributesConverter; +import org.opensearch.telemetry.metrics.tags.Tags; import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.test.OpenSearchTestCase; @@ -20,13 +21,13 @@ public class OTelAttributesConverterTests extends OpenSearchTestCase { public void testConverterNullAttributes() { - io.opentelemetry.api.common.Attributes otelAttributes = OTelAttributesConverter.convert(null); + io.opentelemetry.api.common.Attributes otelAttributes = OTelAttributesConverter.convert((Attributes) null); assertEquals(0, otelAttributes.size()); } public void testConverterEmptyAttributes() { Attributes attributes = Attributes.EMPTY; - io.opentelemetry.api.common.Attributes otelAttributes = OTelAttributesConverter.convert(null); + io.opentelemetry.api.common.Attributes otelAttributes = OTelAttributesConverter.convert(attributes); assertEquals(0, otelAttributes.size()); } @@ -48,4 +49,12 @@ public void testConverterMultipleAttributes() { assertEquals(4, otelAttributes.size()); otelAttributes.asMap().forEach((x, y) -> assertEquals(attributeMap.get(x.getKey()), y)); } + + public void testConverterMultipleTags() { + Tags tags = Tags.create().addTag("key1", 1l).addTag("key2", 1.0).addTag("key3", true).addTag("key4", "value4"); + Map tagsMap = tags.getTagsMap(); + io.opentelemetry.api.common.Attributes otelAttributes = OTelAttributesConverter.convert(tags); + assertEquals(4, otelAttributes.size()); + otelAttributes.asMap().forEach((x, y) -> assertEquals(tagsMap.get(x.getKey()), y)); + } } diff --git a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java index 71ca5fdba2395..6c437911989b8 100644 --- a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java +++ b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java @@ -12,6 +12,7 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; +import org.opensearch.common.unit.TimeValue; /** * Wrapper class to encapsulate tracing related settings @@ -46,6 +47,16 @@ public class TelemetrySettings { Setting.Property.Dynamic ); + /** + * metrics publish interval in seconds. + */ + public static final Setting METRICS_PUBLISH_INTERVAL_SETTING = Setting.timeSetting( + "telemetry.otel.metrics.publish.interval", + TimeValue.timeValueSeconds(60), + Setting.Property.NodeScope, + Setting.Property.Final + ); + private volatile boolean tracingEnabled; private volatile double samplingProbability; diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java b/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java index 9417b90500e88..8a4c859495de6 100644 --- a/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java +++ b/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java @@ -65,7 +65,7 @@ public void close() { private MetricsRegistry metricsRegistry(Optional telemetry) { MetricsRegistry metricsRegistry = telemetry.map(Telemetry::getMetricsTelemetry) .map(metricsTelemetry -> createDefaultMetricsRegistry(metricsTelemetry)) - .map(defaultTracer -> createWrappedMetricsRegistry(defaultTracer)) + .map(defaultMetricsRegistry -> createWrappedMetricsRegistry(defaultMetricsRegistry)) .orElse(NoopMetricsRegistry.INSTANCE); return metricsRegistry; } diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java b/server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java index 9ffa02fb8a2ba..f42467679b2b8 100644 --- a/server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java +++ b/server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java @@ -10,7 +10,7 @@ import org.opensearch.common.annotation.InternalApi; import org.opensearch.telemetry.TelemetrySettings; -import org.opensearch.telemetry.tracing.attributes.Attributes; +import org.opensearch.telemetry.metrics.tags.Tags; /** * Wrapper implementation of {@link Counter}. This delegates call to right {@link Counter} based on the settings @@ -39,9 +39,9 @@ public void add(double value) { } @Override - public void add(double value, Attributes attributes) { + public void add(double value, Tags tags) { if (isMetricsEnabled()) { - delegate.add(value, attributes); + delegate.add(value, tags); } } diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java index 9f589b879f41e..e153f39914449 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java +++ b/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java @@ -15,6 +15,7 @@ import org.opensearch.telemetry.TelemetrySettings; import org.opensearch.telemetry.metrics.Counter; import org.opensearch.telemetry.metrics.WrappedCounter; +import org.opensearch.telemetry.metrics.tags.Tags; import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.test.OpenSearchTestCase; @@ -37,9 +38,9 @@ public void testCounterWithDisabledMetrics() throws Exception { wrappedCounter.add(1.0); verify(mockCounter, never()).add(1.0); - Attributes attributes = Attributes.create().addAttribute("test", "test"); - wrappedCounter.add(1.0, attributes); - verify(mockCounter, never()).add(1.0, attributes); + Tags tags = Tags.create().addTag("test", "test"); + wrappedCounter.add(1.0, tags); + verify(mockCounter, never()).add(1.0, tags); } @@ -52,9 +53,9 @@ public void testCounterWithEnabledMetrics() throws Exception { wrappedCounter.add(1.0); verify(mockCounter).add(1.0); - Attributes attributes = Attributes.create().addAttribute("test", "test"); - wrappedCounter.add(1.0, attributes); - verify(mockCounter).add(1.0, attributes); + Tags tags = Tags.create().addTag("test", "test"); + wrappedCounter.add(1.0, tags); + verify(mockCounter).add(1.0, tags); } private Set> getClusterSettings() { From b7318472c420182d9f94f7b844961d787bde7716 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Tue, 3 Oct 2023 00:24:37 +0530 Subject: [PATCH 07/19] Fix spotless Signed-off-by: Gagan Juneja --- .../telemetry/tracing/metrics/WrappedCounterTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java b/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java index e153f39914449..74ace5b57dd3e 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java +++ b/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java @@ -16,7 +16,6 @@ import org.opensearch.telemetry.metrics.Counter; import org.opensearch.telemetry.metrics.WrappedCounter; import org.opensearch.telemetry.metrics.tags.Tags; -import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.test.OpenSearchTestCase; import java.util.HashSet; From 8794b30a51888c6ef09ec5f9a862b44601a0699d Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Tue, 3 Oct 2023 22:57:29 +0530 Subject: [PATCH 08/19] Address review comment Signed-off-by: Gagan Juneja --- .../opensearch/telemetry/metrics/Counter.java | 3 +++ .../telemetry/metrics/MetricsTelemetry.java | 7 ++++++- .../telemetry/metrics/noop/NoopCounter.java | 3 +++ .../metrics/noop/NoopMetricsRegistry.java | 3 +++ .../telemetry/metrics/noop/package-info.java | 4 ++++ .../telemetry/metrics/tags/package-info.java | 4 ++++ .../telemetry/OTelTelemetryPlugin.java | 2 ++ .../telemetry/metrics/OTelCounter.java | 2 +- .../metrics/OTelMetricsTelemetry.java | 10 ++++----- .../tracing/OTelTracingTelemetry.java | 11 +++++----- .../plugin-metadata/plugin-security.policy | 2 +- .../metrics/OTelMetricsTelemetryTests.java | 21 ++++++++++--------- .../tracing/OTelTracingTelemetryTests.java | 9 ++++---- .../metrics/MetricsRegistryFactoryTests.java | 6 +----- .../metrics/WrappedCounterTests.java | 4 +--- .../metrics/WrappedMetricsRegistryTests.java | 7 +------ 16 files changed, 56 insertions(+), 42 deletions(-) rename server/src/test/java/org/opensearch/telemetry/{tracing => }/metrics/MetricsRegistryFactoryTests.java (91%) rename server/src/test/java/org/opensearch/telemetry/{tracing => }/metrics/WrappedCounterTests.java (94%) rename server/src/test/java/org/opensearch/telemetry/{tracing => }/metrics/WrappedMetricsRegistryTests.java (86%) diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Counter.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Counter.java index 9acfd31d0cebf..c62288d280e2f 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Counter.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/Counter.java @@ -8,11 +8,14 @@ package org.opensearch.telemetry.metrics; +import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.telemetry.metrics.tags.Tags; /** * Counter adds the value to the existing metric. + * {@opensearch.experimental} */ +@ExperimentalApi public interface Counter { /** diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java index bd7f0d5010b0b..f310558ab0124 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java @@ -18,4 +18,9 @@ * @opensearch.experimental */ @ExperimentalApi -public interface MetricsTelemetry extends MetricsRegistry, Closeable {} +public interface MetricsTelemetry extends MetricsRegistry, Closeable { + /** + * closes the resource + */ + void close(); +} diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopCounter.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopCounter.java index de121de3ed465..c1daf564dd3bc 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopCounter.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopCounter.java @@ -8,12 +8,15 @@ package org.opensearch.telemetry.metrics.noop; +import org.opensearch.common.annotation.InternalApi; import org.opensearch.telemetry.metrics.Counter; import org.opensearch.telemetry.metrics.tags.Tags; /** * No-op {@link Counter} + * {@opensearch.internal} */ +@InternalApi public class NoopCounter implements Counter { /** diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopMetricsRegistry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopMetricsRegistry.java index 1836ec8729c3e..640c6842a8960 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopMetricsRegistry.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/NoopMetricsRegistry.java @@ -8,6 +8,7 @@ package org.opensearch.telemetry.metrics.noop; +import org.opensearch.common.annotation.InternalApi; import org.opensearch.telemetry.metrics.Counter; import org.opensearch.telemetry.metrics.MetricsRegistry; @@ -15,7 +16,9 @@ /** *No-op {@link MetricsRegistry} + * {@opensearch.internal} */ +@InternalApi public class NoopMetricsRegistry implements MetricsRegistry { /** diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/package-info.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/package-info.java index 89380bb096538..7c7ed08044993 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/package-info.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/noop/package-info.java @@ -8,5 +8,9 @@ /** * Contains metrics related classes + * {@opensearch.internal} */ +@InternalApi package org.opensearch.telemetry.metrics.noop; + +import org.opensearch.common.annotation.InternalApi; diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/tags/package-info.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/tags/package-info.java index cc184e9a80f40..70bc9be992b32 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/tags/package-info.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/tags/package-info.java @@ -8,5 +8,9 @@ /** * Contains metrics related classes + * @opensearch.experimental */ +@ExperimentalApi package org.opensearch.telemetry.metrics.tags; + +import org.opensearch.common.annotation.ExperimentalApi; diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java index 9692f59a413a3..5aa31a86bf4e8 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java @@ -28,6 +28,8 @@ */ public class OTelTelemetryPlugin extends Plugin implements TelemetryPlugin { + public static final String INSTRUMENTATION_SCOPE_NAME = "org.opensearch.telemetry"; + static final String OTEL_TRACER_NAME = "otel"; private final Settings settings; diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelCounter.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelCounter.java index d42c6720afea6..b72f63e027243 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelCounter.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelCounter.java @@ -16,7 +16,7 @@ /** * OTel Counter */ -public class OTelCounter implements Counter { +class OTelCounter implements Counter { private final DoubleCounter otelDoubleCounter; diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java index 3c9d0e239696b..bc0b144e63ad5 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java @@ -10,9 +10,8 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.telemetry.OTelTelemetryPlugin; -import java.io.Closeable; -import java.io.IOException; import java.security.AccessController; import java.security.PrivilegedAction; @@ -20,6 +19,7 @@ import io.opentelemetry.api.metrics.DoubleCounter; import io.opentelemetry.api.metrics.DoubleUpDownCounter; import io.opentelemetry.api.metrics.Meter; +import io.opentelemetry.sdk.OpenTelemetrySdk; /** * OTel implementation for {@link MetricsTelemetry} @@ -35,7 +35,7 @@ public class OTelMetricsTelemetry implements MetricsTelemetry { */ public OTelMetricsTelemetry(OpenTelemetry openTelemetry) { this.openTelemetry = openTelemetry; - this.otelMeter = openTelemetry.getMeter("os-meter"); + this.otelMeter = openTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); } @Override @@ -66,8 +66,8 @@ public Counter createUpDownCounter(String name, String description, String unit) public void close() { // There is no harm closing the openTelemetry multiple times. try { - ((Closeable) openTelemetry).close(); - } catch (IOException e) { + ((OpenTelemetrySdk) openTelemetry).getSdkMeterProvider().close(); + } catch (Exception e) { logger.warn("Error while closing Opentelemetry", e); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index 726205f8f8573..6d3fc6f11404b 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -11,12 +11,11 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.telemetry.OTelAttributesConverter; - -import java.io.Closeable; -import java.io.IOException; +import org.opensearch.telemetry.OTelTelemetryPlugin; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.context.Context; +import io.opentelemetry.sdk.OpenTelemetrySdk; /** * OTel based Telemetry provider @@ -33,15 +32,15 @@ public class OTelTracingTelemetry implements TracingTelemetry { */ public OTelTracingTelemetry(OpenTelemetry openTelemetry) { this.openTelemetry = openTelemetry; - this.otelTracer = openTelemetry.getTracer("os-tracer"); + this.otelTracer = openTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); } @Override public void close() { try { - ((Closeable) openTelemetry).close(); - } catch (IOException e) { + ((OpenTelemetrySdk) openTelemetry).getSdkTracerProvider().close(); + } catch (Exception e) { logger.warn("Error while closing Opentelemetry", e); } } diff --git a/plugins/telemetry-otel/src/main/plugin-metadata/plugin-security.policy b/plugins/telemetry-otel/src/main/plugin-metadata/plugin-security.policy index 9d529ed5a2a56..077822b7d3cfc 100644 --- a/plugins/telemetry-otel/src/main/plugin-metadata/plugin-security.policy +++ b/plugins/telemetry-otel/src/main/plugin-metadata/plugin-security.policy @@ -11,7 +11,7 @@ grant { permission java.lang.RuntimePermission "accessDeclaredMembers"; permission java.net.NetPermission "getProxySelector"; permission java.net.SocketPermission "*", "connect,resolve"; - permission java.util.PropertyPermission "*", "read,write"; + permission java.util.PropertyPermission "otel.experimental.sdk.metrics.debug", "read,write"; }; diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java index db8e516350395..c0780fe309908 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java @@ -9,7 +9,8 @@ package org.opensearch.telemetry.metrics; import org.opensearch.telemetry.OTelAttributesConverter; -import org.opensearch.telemetry.tracing.attributes.Attributes; +import org.opensearch.telemetry.OTelTelemetryPlugin; +import org.opensearch.telemetry.metrics.tags.Tags; import org.opensearch.test.OpenSearchTestCase; import io.opentelemetry.api.OpenTelemetry; @@ -37,7 +38,7 @@ public void testCounter() { LongCounterBuilder mockOTelLongCounterBuilder = mock(LongCounterBuilder.class); DoubleCounterBuilder mockOTelDoubleCounterBuilder = mock(DoubleCounterBuilder.class); - when(mockOpenTelemetry.getMeter("os-meter")).thenReturn(mockMeter); + when(mockOpenTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry); when(mockMeter.counterBuilder(counterName)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setDescription(description)).thenReturn(mockOTelLongCounterBuilder); @@ -48,9 +49,9 @@ public void testCounter() { Counter counter = metricsTelemetry.createCounter(counterName, description, unit); counter.add(1.0); verify(mockOTelDoubleCounter).add(1.0); - Attributes attributes = Attributes.create().addAttribute("test", "test"); - counter.add(2.0, attributes); - verify(mockOTelDoubleCounter).add(2.0, OTelAttributesConverter.convert(attributes)); + Tags tags = Tags.create().addTag("test", "test"); + counter.add(2.0, tags); + verify(mockOTelDoubleCounter).add(2.0, OTelAttributesConverter.convert(tags)); } public void testCounterNegativeValue() { @@ -63,7 +64,7 @@ public void testCounterNegativeValue() { LongCounterBuilder mockOTelLongCounterBuilder = mock(LongCounterBuilder.class); DoubleCounterBuilder mockOTelDoubleCounterBuilder = mock(DoubleCounterBuilder.class); - when(mockOpenTelemetry.getMeter("os-meter")).thenReturn(mockMeter); + when(mockOpenTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry); when(mockMeter.counterBuilder(counterName)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setDescription(description)).thenReturn(mockOTelLongCounterBuilder); @@ -86,7 +87,7 @@ public void testUpDownCounter() { LongUpDownCounterBuilder mockOTelLongUpDownCounterBuilder = mock(LongUpDownCounterBuilder.class); DoubleUpDownCounterBuilder mockOTelDoubleUpDownCounterBuilder = mock(DoubleUpDownCounterBuilder.class); - when(mockOpenTelemetry.getMeter("os-meter")).thenReturn(mockMeter); + when(mockOpenTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry); when(mockMeter.upDownCounterBuilder(counterName)).thenReturn(mockOTelLongUpDownCounterBuilder); when(mockOTelLongUpDownCounterBuilder.setDescription(description)).thenReturn(mockOTelLongUpDownCounterBuilder); @@ -97,8 +98,8 @@ public void testUpDownCounter() { Counter counter = metricsTelemetry.createUpDownCounter(counterName, description, unit); counter.add(1.0); verify(mockOTelUpDownDoubleCounter).add(1.0); - Attributes attributes = Attributes.create().addAttribute("test", "test"); - counter.add(-2.0, attributes); - verify(mockOTelUpDownDoubleCounter).add((-2.0), OTelAttributesConverter.convert(attributes)); + Tags tags = Tags.create().addTag("test", "test"); + counter.add(-2.0, tags); + verify(mockOTelUpDownDoubleCounter).add((-2.0), OTelAttributesConverter.convert(tags)); } } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java index 505756318ff62..0181033bcc9c7 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java @@ -8,6 +8,7 @@ package org.opensearch.telemetry.tracing; +import org.opensearch.telemetry.OTelTelemetryPlugin; import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.test.OpenSearchTestCase; @@ -30,7 +31,7 @@ public class OTelTracingTelemetryTests extends OpenSearchTestCase { public void testCreateSpanWithoutParent() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - when(mockOpenTelemetry.getTracer("os-tracer")).thenReturn(mockTracer); + when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); when(mockSpanBuilder.setAllAttributes(any(io.opentelemetry.api.common.Attributes.class))).thenReturn(mockSpanBuilder); @@ -48,7 +49,7 @@ public void testCreateSpanWithoutParent() { public void testCreateSpanWithParent() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - when(mockOpenTelemetry.getTracer("os-tracer")).thenReturn(mockTracer); + when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); when(mockSpanBuilder.setParent(any())).thenReturn(mockSpanBuilder); @@ -72,7 +73,7 @@ public void testCreateSpanWithParent() { public void testCreateSpanWithParentWithMultipleAttributes() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - when(mockOpenTelemetry.getTracer("os-tracer")).thenReturn(mockTracer); + when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); when(mockSpanBuilder.setParent(any())).thenReturn(mockSpanBuilder); @@ -118,7 +119,7 @@ private io.opentelemetry.api.common.Attributes createAttributeLong(Attributes at public void testGetContextPropagator() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - when(mockOpenTelemetry.getTracer("os-tracer")).thenReturn(mockTracer); + when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/MetricsRegistryFactoryTests.java b/server/src/test/java/org/opensearch/telemetry/metrics/MetricsRegistryFactoryTests.java similarity index 91% rename from server/src/test/java/org/opensearch/telemetry/tracing/metrics/MetricsRegistryFactoryTests.java rename to server/src/test/java/org/opensearch/telemetry/metrics/MetricsRegistryFactoryTests.java index 656e94406b022..6094d280c20a9 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/MetricsRegistryFactoryTests.java +++ b/server/src/test/java/org/opensearch/telemetry/metrics/MetricsRegistryFactoryTests.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.telemetry.tracing.metrics; +package org.opensearch.telemetry.metrics; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; @@ -14,10 +14,6 @@ import org.opensearch.common.util.FeatureFlags; import org.opensearch.telemetry.Telemetry; import org.opensearch.telemetry.TelemetrySettings; -import org.opensearch.telemetry.metrics.MetricsRegistry; -import org.opensearch.telemetry.metrics.MetricsRegistryFactory; -import org.opensearch.telemetry.metrics.MetricsTelemetry; -import org.opensearch.telemetry.metrics.WrappedMetricsRegistry; import org.opensearch.telemetry.metrics.noop.NoopCounter; import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry; import org.opensearch.telemetry.tracing.TracingTelemetry; diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java b/server/src/test/java/org/opensearch/telemetry/metrics/WrappedCounterTests.java similarity index 94% rename from server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java rename to server/src/test/java/org/opensearch/telemetry/metrics/WrappedCounterTests.java index 74ace5b57dd3e..e01d200bb8527 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedCounterTests.java +++ b/server/src/test/java/org/opensearch/telemetry/metrics/WrappedCounterTests.java @@ -6,15 +6,13 @@ * compatible open source license. */ -package org.opensearch.telemetry.tracing.metrics; +package org.opensearch.telemetry.metrics; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; import org.opensearch.telemetry.TelemetrySettings; -import org.opensearch.telemetry.metrics.Counter; -import org.opensearch.telemetry.metrics.WrappedCounter; import org.opensearch.telemetry.metrics.tags.Tags; import org.opensearch.test.OpenSearchTestCase; diff --git a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedMetricsRegistryTests.java b/server/src/test/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistryTests.java similarity index 86% rename from server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedMetricsRegistryTests.java rename to server/src/test/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistryTests.java index 9c0bceed3c8e6..119f6536b5d2e 100644 --- a/server/src/test/java/org/opensearch/telemetry/tracing/metrics/WrappedMetricsRegistryTests.java +++ b/server/src/test/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistryTests.java @@ -6,18 +6,13 @@ * compatible open source license. */ -package org.opensearch.telemetry.tracing.metrics; +package org.opensearch.telemetry.metrics; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.FeatureFlags; import org.opensearch.telemetry.TelemetrySettings; -import org.opensearch.telemetry.metrics.Counter; -import org.opensearch.telemetry.metrics.DefaultMetricsRegistry; -import org.opensearch.telemetry.metrics.MetricsTelemetry; -import org.opensearch.telemetry.metrics.WrappedCounter; -import org.opensearch.telemetry.metrics.WrappedMetricsRegistry; import org.opensearch.test.OpenSearchTestCase; import java.util.HashSet; From 8cae5323f7b5d2d7eb4ddaa3063eae8bfa62f617 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Wed, 4 Oct 2023 01:14:47 +0530 Subject: [PATCH 09/19] Address review comment Signed-off-by: Gagan Juneja --- .../telemetry/OTelTelemetryPlugin.java | 4 +- .../metrics/OTelMetricsTelemetry.java | 7 +-- .../tracing/OTelResourceProvider.java | 16 +++-- .../tracing/OTelTracingTelemetry.java | 7 +-- .../telemetry/OTelTelemetryPluginTests.java | 6 +- .../metrics/OTelMetricsTelemetryTests.java | 8 +-- .../tracing/OTelTracingTelemetryTests.java | 10 +-- .../sampler/ProbabilisticSamplerTests.java | 5 +- .../common/settings/ClusterSettings.java | 6 +- .../telemetry/TelemetrySettings.java | 28 --------- .../metrics/MetricsRegistryFactory.java | 5 -- .../telemetry/metrics/WrappedCounter.java | 51 --------------- .../metrics/WrappedMetricsRegistry.java | 49 --------------- .../metrics/MetricsRegistryFactoryTests.java | 6 +- .../metrics/WrappedCounterTests.java | 63 ------------------- .../metrics/WrappedMetricsRegistryTests.java | 49 --------------- 16 files changed, 34 insertions(+), 286 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java delete mode 100644 server/src/main/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistry.java delete mode 100644 server/src/test/java/org/opensearch/telemetry/metrics/WrappedCounterTests.java delete mode 100644 server/src/test/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistryTests.java diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java index 5aa31a86bf4e8..b007cc60e304d 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java @@ -21,7 +21,7 @@ import java.util.List; import java.util.Optional; -import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.sdk.OpenTelemetrySdk; /** * Telemetry plugin based on Otel @@ -64,7 +64,7 @@ public String getName() { } private Telemetry telemetry(TelemetrySettings telemetrySettings) { - final OpenTelemetry openTelemetry = OTelResourceProvider.get(telemetrySettings, settings); + final OpenTelemetrySdk openTelemetry = OTelResourceProvider.get(telemetrySettings, settings); return new OTelTelemetry(new OTelTracingTelemetry(openTelemetry), new OTelMetricsTelemetry(openTelemetry)); } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java index bc0b144e63ad5..5538f5b3625e4 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java @@ -15,7 +15,6 @@ import java.security.AccessController; import java.security.PrivilegedAction; -import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.metrics.DoubleCounter; import io.opentelemetry.api.metrics.DoubleUpDownCounter; import io.opentelemetry.api.metrics.Meter; @@ -26,14 +25,14 @@ */ public class OTelMetricsTelemetry implements MetricsTelemetry { private static final Logger logger = LogManager.getLogger(OTelMetricsTelemetry.class); - private final OpenTelemetry openTelemetry; + private final OpenTelemetrySdk openTelemetry; private final Meter otelMeter; /** * Constructor. * @param openTelemetry telemetry. */ - public OTelMetricsTelemetry(OpenTelemetry openTelemetry) { + public OTelMetricsTelemetry(OpenTelemetrySdk openTelemetry) { this.openTelemetry = openTelemetry; this.otelMeter = openTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); } @@ -66,7 +65,7 @@ public Counter createUpDownCounter(String name, String description, String unit) public void close() { // There is no harm closing the openTelemetry multiple times. try { - ((OpenTelemetrySdk) openTelemetry).getSdkMeterProvider().close(); + openTelemetry.getSdkMeterProvider().close(); } catch (Exception e) { logger.warn("Error while closing Opentelemetry", e); } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java index d436264bd0557..a6a1f12aab8a9 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelResourceProvider.java @@ -19,7 +19,6 @@ import java.security.PrivilegedAction; import java.util.concurrent.TimeUnit; -import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; import io.opentelemetry.context.propagation.ContextPropagators; @@ -47,11 +46,11 @@ private OTelResourceProvider() {} * Creates OpenTelemetry instance with default configuration * @param telemetrySettings telemetry settings * @param settings cluster settings - * @return OpenTelemetry instance + * @return OpenTelemetrySdk instance */ - public static OpenTelemetry get(TelemetrySettings telemetrySettings, Settings settings) { + public static OpenTelemetrySdk get(TelemetrySettings telemetrySettings, Settings settings) { return AccessController.doPrivileged( - (PrivilegedAction) () -> get( + (PrivilegedAction) () -> get( settings, OTelSpanExporterFactory.create(settings), ContextPropagators.create(W3CTraceContextPropagator.getInstance()), @@ -66,9 +65,14 @@ public static OpenTelemetry get(TelemetrySettings telemetrySettings, Settings se * @param spanExporter span exporter instance * @param contextPropagators context propagator instance * @param sampler sampler instance - * @return Opentelemetry instance + * @return OpenTelemetrySdk instance */ - public static OpenTelemetry get(Settings settings, SpanExporter spanExporter, ContextPropagators contextPropagators, Sampler sampler) { + public static OpenTelemetrySdk get( + Settings settings, + SpanExporter spanExporter, + ContextPropagators contextPropagators, + Sampler sampler + ) { Resource resource = Resource.create(Attributes.of(ResourceAttributes.SERVICE_NAME, "OpenSearch")); SdkTracerProvider sdkTracerProvider = createSdkTracerProvider(settings, spanExporter, sampler, resource); SdkMeterProvider sdkMeterProvider = createSdkMetricProvider(settings, resource); diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index 6d3fc6f11404b..ea7bce5080e76 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -13,7 +13,6 @@ import org.opensearch.telemetry.OTelAttributesConverter; import org.opensearch.telemetry.OTelTelemetryPlugin; -import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.context.Context; import io.opentelemetry.sdk.OpenTelemetrySdk; @@ -23,14 +22,14 @@ public class OTelTracingTelemetry implements TracingTelemetry { private static final Logger logger = LogManager.getLogger(OTelTracingTelemetry.class); - private final OpenTelemetry openTelemetry; + private final OpenTelemetrySdk openTelemetry; private final io.opentelemetry.api.trace.Tracer otelTracer; /** * Creates OTel based Telemetry * @param openTelemetry OpenTelemetry instance */ - public OTelTracingTelemetry(OpenTelemetry openTelemetry) { + public OTelTracingTelemetry(OpenTelemetrySdk openTelemetry) { this.openTelemetry = openTelemetry; this.otelTracer = openTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); @@ -39,7 +38,7 @@ public OTelTracingTelemetry(OpenTelemetry openTelemetry) { @Override public void close() { try { - ((OpenTelemetrySdk) openTelemetry).getSdkTracerProvider().close(); + openTelemetry.getSdkTracerProvider().close(); } catch (Exception e) { logger.warn("Error while closing Opentelemetry", e); } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java index 795cdeb2ebc56..2a6b9d687d2d9 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java @@ -32,7 +32,6 @@ import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_BATCH_SIZE_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_MAX_QUEUE_SIZE_SETTING; -import static org.opensearch.telemetry.TelemetrySettings.METRICS_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_PROBABILITY; @@ -51,10 +50,7 @@ public void setup() { Settings settings = Settings.builder().put(TRACER_EXPORTER_DELAY_SETTING.getKey(), "1s").build(); oTelTelemetryModulePlugin = new OTelTelemetryPlugin(settings); telemetry = oTelTelemetryModulePlugin.getTelemetry( - new TelemetrySettings( - Settings.EMPTY, - new ClusterSettings(settings, Set.of(TRACER_ENABLED_SETTING, TRACER_SAMPLER_PROBABILITY, METRICS_ENABLED_SETTING)) - ) + new TelemetrySettings(Settings.EMPTY, new ClusterSettings(settings, Set.of(TRACER_ENABLED_SETTING, TRACER_SAMPLER_PROBABILITY))) ); tracingTelemetry = telemetry.get().getTracingTelemetry(); metricsTelemetry = telemetry.get().getMetricsTelemetry(); diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java index c0780fe309908..cf398e3467401 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java @@ -13,7 +13,6 @@ import org.opensearch.telemetry.metrics.tags.Tags; import org.opensearch.test.OpenSearchTestCase; -import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.metrics.DoubleCounter; import io.opentelemetry.api.metrics.DoubleCounterBuilder; import io.opentelemetry.api.metrics.DoubleUpDownCounter; @@ -21,6 +20,7 @@ import io.opentelemetry.api.metrics.LongCounterBuilder; import io.opentelemetry.api.metrics.LongUpDownCounterBuilder; import io.opentelemetry.api.metrics.Meter; +import io.opentelemetry.sdk.OpenTelemetrySdk; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -32,7 +32,7 @@ public void testCounter() { String counterName = "test-counter"; String description = "test"; String unit = "1"; - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Meter mockMeter = mock(Meter.class); DoubleCounter mockOTelDoubleCounter = mock(DoubleCounter.class); LongCounterBuilder mockOTelLongCounterBuilder = mock(LongCounterBuilder.class); @@ -58,7 +58,7 @@ public void testCounterNegativeValue() { String counterName = "test-counter"; String description = "test"; String unit = "1"; - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Meter mockMeter = mock(Meter.class); DoubleCounter mockOTelDoubleCounter = mock(DoubleCounter.class); LongCounterBuilder mockOTelLongCounterBuilder = mock(LongCounterBuilder.class); @@ -81,7 +81,7 @@ public void testUpDownCounter() { String counterName = "test-counter"; String description = "test"; String unit = "1"; - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Meter mockMeter = mock(Meter.class); DoubleUpDownCounter mockOTelUpDownDoubleCounter = mock(DoubleUpDownCounter.class); LongUpDownCounterBuilder mockOTelLongUpDownCounterBuilder = mock(LongUpDownCounterBuilder.class); diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java index 0181033bcc9c7..d9d66f4ab4098 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java @@ -15,10 +15,10 @@ import java.util.Collections; import java.util.Map; -import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.sdk.OpenTelemetrySdk; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -29,7 +29,7 @@ public class OTelTracingTelemetryTests extends OpenSearchTestCase { public void testCreateSpanWithoutParent() { - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Tracer mockTracer = mock(Tracer.class); when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); @@ -47,7 +47,7 @@ public void testCreateSpanWithoutParent() { } public void testCreateSpanWithParent() { - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Tracer mockTracer = mock(Tracer.class); when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); @@ -71,7 +71,7 @@ public void testCreateSpanWithParent() { } public void testCreateSpanWithParentWithMultipleAttributes() { - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Tracer mockTracer = mock(Tracer.class); when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); @@ -117,7 +117,7 @@ private io.opentelemetry.api.common.Attributes createAttributeLong(Attributes at } public void testGetContextPropagator() { - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); Tracer mockTracer = mock(Tracer.class); when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSamplerTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSamplerTests.java index 5cf7cf127822c..639dc341ef0db 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSamplerTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/sampler/ProbabilisticSamplerTests.java @@ -18,7 +18,6 @@ import io.opentelemetry.sdk.trace.samplers.Sampler; import static org.opensearch.telemetry.OTelTelemetrySettings.TRACER_EXPORTER_DELAY_SETTING; -import static org.opensearch.telemetry.TelemetrySettings.METRICS_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_ENABLED_SETTING; import static org.opensearch.telemetry.TelemetrySettings.TRACER_SAMPLER_PROBABILITY; @@ -34,7 +33,7 @@ public void testDefaultGetSampler() { Settings settings = Settings.builder().put(TRACER_EXPORTER_DELAY_SETTING.getKey(), "1s").build(); TelemetrySettings telemetrySettings = new TelemetrySettings( Settings.EMPTY, - new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING, METRICS_ENABLED_SETTING)) + new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)) ); // Probabilistic Sampler @@ -48,7 +47,7 @@ public void testGetSamplerWithUpdatedSamplingRatio() { Settings settings = Settings.builder().put(TRACER_EXPORTER_DELAY_SETTING.getKey(), "1s").build(); TelemetrySettings telemetrySettings = new TelemetrySettings( Settings.EMPTY, - new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING, METRICS_ENABLED_SETTING)) + new ClusterSettings(settings, Set.of(TRACER_SAMPLER_PROBABILITY, TRACER_ENABLED_SETTING)) ); // Probabilistic Sampler diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 58e56575ab881..4cd3490cffb4c 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -694,10 +694,6 @@ public void apply(Settings value, Settings current, Settings previous) { SearchService.CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_SETTING ), List.of(FeatureFlags.TELEMETRY), - List.of( - TelemetrySettings.TRACER_ENABLED_SETTING, - TelemetrySettings.TRACER_SAMPLER_PROBABILITY, - TelemetrySettings.METRICS_ENABLED_SETTING - ) + List.of(TelemetrySettings.TRACER_ENABLED_SETTING, TelemetrySettings.TRACER_SAMPLER_PROBABILITY) ); } diff --git a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java index 6c437911989b8..edb20cfa9dfc5 100644 --- a/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java +++ b/server/src/main/java/org/opensearch/telemetry/TelemetrySettings.java @@ -28,13 +28,6 @@ public class TelemetrySettings { Setting.Property.Dynamic ); - public static final Setting METRICS_ENABLED_SETTING = Setting.boolSetting( - "telemetry.metrics.enabled", - false, - Setting.Property.NodeScope, - Setting.Property.Dynamic - ); - /** * Probability of sampler */ @@ -60,16 +53,12 @@ public class TelemetrySettings { private volatile boolean tracingEnabled; private volatile double samplingProbability; - private volatile boolean metricsEnabled; - public TelemetrySettings(Settings settings, ClusterSettings clusterSettings) { this.tracingEnabled = TRACER_ENABLED_SETTING.get(settings); this.samplingProbability = TRACER_SAMPLER_PROBABILITY.get(settings); - this.metricsEnabled = METRICS_ENABLED_SETTING.get(settings); clusterSettings.addSettingsUpdateConsumer(TRACER_ENABLED_SETTING, this::setTracingEnabled); clusterSettings.addSettingsUpdateConsumer(TRACER_SAMPLER_PROBABILITY, this::setSamplingProbability); - clusterSettings.addSettingsUpdateConsumer(METRICS_ENABLED_SETTING, this::setMetricsEnabled); } public void setTracingEnabled(boolean tracingEnabled) { @@ -94,21 +83,4 @@ public void setSamplingProbability(double samplingProbability) { public double getSamplingProbability() { return samplingProbability; } - - /** - * update the metrics enabled property. - * @param metricsEnabled metrics enabled. - */ - public void setMetricsEnabled(boolean metricsEnabled) { - this.metricsEnabled = metricsEnabled; - } - - /** - * Returns whether metrics are enabled or not. - * @return enabled/disabled flag. - */ - public boolean isMetricsEnabled() { - return metricsEnabled; - } - } diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java b/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java index 8a4c859495de6..c7e2229c18437 100644 --- a/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java +++ b/server/src/main/java/org/opensearch/telemetry/metrics/MetricsRegistryFactory.java @@ -65,7 +65,6 @@ public void close() { private MetricsRegistry metricsRegistry(Optional telemetry) { MetricsRegistry metricsRegistry = telemetry.map(Telemetry::getMetricsTelemetry) .map(metricsTelemetry -> createDefaultMetricsRegistry(metricsTelemetry)) - .map(defaultMetricsRegistry -> createWrappedMetricsRegistry(defaultMetricsRegistry)) .orElse(NoopMetricsRegistry.INSTANCE); return metricsRegistry; } @@ -74,8 +73,4 @@ private MetricsRegistry createDefaultMetricsRegistry(MetricsTelemetry metricsTel return new DefaultMetricsRegistry(metricsTelemetry); } - private MetricsRegistry createWrappedMetricsRegistry(MetricsRegistry metricsRegistry) { - return new WrappedMetricsRegistry(telemetrySettings, metricsRegistry); - } - } diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java b/server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java deleted file mode 100644 index f42467679b2b8..0000000000000 --- a/server/src/main/java/org/opensearch/telemetry/metrics/WrappedCounter.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.telemetry.metrics; - -import org.opensearch.common.annotation.InternalApi; -import org.opensearch.telemetry.TelemetrySettings; -import org.opensearch.telemetry.metrics.tags.Tags; - -/** - * Wrapper implementation of {@link Counter}. This delegates call to right {@link Counter} based on the settings - */ -@InternalApi -public class WrappedCounter implements Counter { - - private final TelemetrySettings telemetrySettings; - private final Counter delegate; - - /** - * Constructor - * @param telemetrySettings telemetry settings. - * @param delegate delegate counter. - */ - public WrappedCounter(TelemetrySettings telemetrySettings, Counter delegate) { - this.telemetrySettings = telemetrySettings; - this.delegate = delegate; - } - - @Override - public void add(double value) { - if (isMetricsEnabled()) { - delegate.add(value); - } - } - - @Override - public void add(double value, Tags tags) { - if (isMetricsEnabled()) { - delegate.add(value, tags); - } - } - - private boolean isMetricsEnabled() { - return telemetrySettings.isMetricsEnabled(); - } -} diff --git a/server/src/main/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistry.java b/server/src/main/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistry.java deleted file mode 100644 index c83ef21501d4e..0000000000000 --- a/server/src/main/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistry.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.telemetry.metrics; - -import org.opensearch.common.annotation.InternalApi; -import org.opensearch.telemetry.TelemetrySettings; - -import java.io.IOException; - -/** - * Wrapper implementation of {@link MetricsRegistry}. This delegates call to right {@link MetricsRegistry} based on the settings - */ -@InternalApi -public class WrappedMetricsRegistry implements MetricsRegistry { - - private final MetricsRegistry defaultMetricsRegistry; - private final TelemetrySettings telemetrySettings; - - /** - * Constructor. - * @param defaultMetricsRegistry default meter registry. - * @param telemetrySettings telemetry settings. - */ - public WrappedMetricsRegistry(TelemetrySettings telemetrySettings, MetricsRegistry defaultMetricsRegistry) { - this.telemetrySettings = telemetrySettings; - this.defaultMetricsRegistry = defaultMetricsRegistry; - } - - @Override - public Counter createCounter(String name, String description, String unit) { - return new WrappedCounter(telemetrySettings, defaultMetricsRegistry.createCounter(name, description, unit)); - } - - @Override - public Counter createUpDownCounter(String name, String description, String unit) { - return new WrappedCounter(telemetrySettings, defaultMetricsRegistry.createUpDownCounter(name, description, unit)); - } - - @Override - public void close() throws IOException { - defaultMetricsRegistry.close(); - } -} diff --git a/server/src/test/java/org/opensearch/telemetry/metrics/MetricsRegistryFactoryTests.java b/server/src/test/java/org/opensearch/telemetry/metrics/MetricsRegistryFactoryTests.java index 6094d280c20a9..5d5ea62dd161e 100644 --- a/server/src/test/java/org/opensearch/telemetry/metrics/MetricsRegistryFactoryTests.java +++ b/server/src/test/java/org/opensearch/telemetry/metrics/MetricsRegistryFactoryTests.java @@ -37,7 +37,7 @@ public void close() { metricsRegistryFactory.close(); } - public void testGetMeterRegistrywithUnavailableMetricsTelemetry() { + public void testGetMeterRegistryWithUnavailableMetricsTelemetry() { Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), false).build(); TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); Telemetry mockTelemetry = mock(Telemetry.class); @@ -51,7 +51,7 @@ public void testGetMeterRegistrywithUnavailableMetricsTelemetry() { assertTrue(metricsRegistry.createUpDownCounter("test", "test", "test") == NoopCounter.INSTANCE); } - public void testGetTracerWithAvailableTracingTelemetryReturnsWrappedTracer() { + public void testGetMetricsWithAvailableMetricsTelemetry() { Settings settings = Settings.builder().put(TelemetrySettings.TRACER_ENABLED_SETTING.getKey(), true).build(); TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); Telemetry mockTelemetry = mock(Telemetry.class); @@ -59,7 +59,7 @@ public void testGetTracerWithAvailableTracingTelemetryReturnsWrappedTracer() { metricsRegistryFactory = new MetricsRegistryFactory(telemetrySettings, Optional.of(mockTelemetry)); MetricsRegistry metricsRegistry = metricsRegistryFactory.getMetricsRegistry(); - assertTrue(metricsRegistry instanceof WrappedMetricsRegistry); + assertTrue(metricsRegistry instanceof DefaultMetricsRegistry); } diff --git a/server/src/test/java/org/opensearch/telemetry/metrics/WrappedCounterTests.java b/server/src/test/java/org/opensearch/telemetry/metrics/WrappedCounterTests.java deleted file mode 100644 index e01d200bb8527..0000000000000 --- a/server/src/test/java/org/opensearch/telemetry/metrics/WrappedCounterTests.java +++ /dev/null @@ -1,63 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.telemetry.metrics; - -import org.opensearch.common.settings.ClusterSettings; -import org.opensearch.common.settings.Setting; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; -import org.opensearch.telemetry.TelemetrySettings; -import org.opensearch.telemetry.metrics.tags.Tags; -import org.opensearch.test.OpenSearchTestCase; - -import java.util.HashSet; -import java.util.List; -import java.util.Set; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; - -public class WrappedCounterTests extends OpenSearchTestCase { - - public void testCounterWithDisabledMetrics() throws Exception { - Settings settings = Settings.builder().put(TelemetrySettings.METRICS_ENABLED_SETTING.getKey(), false).build(); - TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); - Counter mockCounter = mock(Counter.class); - - WrappedCounter wrappedCounter = new WrappedCounter(telemetrySettings, mockCounter); - wrappedCounter.add(1.0); - verify(mockCounter, never()).add(1.0); - - Tags tags = Tags.create().addTag("test", "test"); - wrappedCounter.add(1.0, tags); - verify(mockCounter, never()).add(1.0, tags); - - } - - public void testCounterWithEnabledMetrics() throws Exception { - Settings settings = Settings.builder().put(TelemetrySettings.METRICS_ENABLED_SETTING.getKey(), true).build(); - TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); - Counter mockCounter = mock(Counter.class); - - WrappedCounter wrappedCounter = new WrappedCounter(telemetrySettings, mockCounter); - wrappedCounter.add(1.0); - verify(mockCounter).add(1.0); - - Tags tags = Tags.create().addTag("test", "test"); - wrappedCounter.add(1.0, tags); - verify(mockCounter).add(1.0, tags); - } - - private Set> getClusterSettings() { - Set> allTracerSettings = new HashSet<>(); - ClusterSettings.FEATURE_FLAGGED_CLUSTER_SETTINGS.get(List.of(FeatureFlags.TELEMETRY)).stream().forEach((allTracerSettings::add)); - return allTracerSettings; - } -} diff --git a/server/src/test/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistryTests.java b/server/src/test/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistryTests.java deleted file mode 100644 index 119f6536b5d2e..0000000000000 --- a/server/src/test/java/org/opensearch/telemetry/metrics/WrappedMetricsRegistryTests.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.telemetry.metrics; - -import org.opensearch.common.settings.ClusterSettings; -import org.opensearch.common.settings.Setting; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; -import org.opensearch.telemetry.TelemetrySettings; -import org.opensearch.test.OpenSearchTestCase; - -import java.util.HashSet; -import java.util.List; -import java.util.Set; - -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -public class WrappedMetricsRegistryTests extends OpenSearchTestCase { - - public void testCounter() throws Exception { - Settings settings = Settings.builder().put(TelemetrySettings.METRICS_ENABLED_SETTING.getKey(), false).build(); - TelemetrySettings telemetrySettings = new TelemetrySettings(settings, new ClusterSettings(settings, getClusterSettings())); - MetricsTelemetry mockMetricsTelemetry = mock(MetricsTelemetry.class); - Counter mockCounter = mock(Counter.class); - Counter mockUpDownCounter = mock(Counter.class); - DefaultMetricsRegistry defaultMeterRegistry = new DefaultMetricsRegistry(mockMetricsTelemetry); - - when(mockMetricsTelemetry.createCounter("test", "test", "test")).thenReturn(mockCounter); - when(mockMetricsTelemetry.createUpDownCounter("test", "test", "test")).thenReturn(mockUpDownCounter); - - WrappedMetricsRegistry wrappedMeterRegistry = new WrappedMetricsRegistry(telemetrySettings, defaultMeterRegistry); - assertTrue(wrappedMeterRegistry.createCounter("test", "test", "test") instanceof WrappedCounter); - assertTrue(wrappedMeterRegistry.createUpDownCounter("test", "test", "test") instanceof WrappedCounter); - } - - private Set> getClusterSettings() { - Set> allTracerSettings = new HashSet<>(); - ClusterSettings.FEATURE_FLAGGED_CLUSTER_SETTINGS.get(List.of(FeatureFlags.TELEMETRY)).stream().forEach((allTracerSettings::add)); - return allTracerSettings; - } - -} From 5a43bb697fdc5794a904585412b1eb510993efb7 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Wed, 4 Oct 2023 01:39:47 +0530 Subject: [PATCH 10/19] Fix java doc Signed-off-by: Gagan Juneja --- .../java/org/opensearch/telemetry/OTelTelemetryPlugin.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java index b007cc60e304d..e8fc3fe53b1ef 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java @@ -28,6 +28,9 @@ */ public class OTelTelemetryPlugin extends Plugin implements TelemetryPlugin { + /** + * Instrumentation scope name. + */ public static final String INSTRUMENTATION_SCOPE_NAME = "org.opensearch.telemetry"; static final String OTEL_TRACER_NAME = "otel"; From 7a098c3c985b1d86035f18cf11da5071ac4ecbc7 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Wed, 4 Oct 2023 01:59:30 +0530 Subject: [PATCH 11/19] Fix security access issue Signed-off-by: Gagan Juneja --- .../src/main/plugin-metadata/plugin-security.policy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/telemetry-otel/src/main/plugin-metadata/plugin-security.policy b/plugins/telemetry-otel/src/main/plugin-metadata/plugin-security.policy index 077822b7d3cfc..9d529ed5a2a56 100644 --- a/plugins/telemetry-otel/src/main/plugin-metadata/plugin-security.policy +++ b/plugins/telemetry-otel/src/main/plugin-metadata/plugin-security.policy @@ -11,7 +11,7 @@ grant { permission java.lang.RuntimePermission "accessDeclaredMembers"; permission java.net.NetPermission "getProxySelector"; permission java.net.SocketPermission "*", "connect,resolve"; - permission java.util.PropertyPermission "otel.experimental.sdk.metrics.debug", "read,write"; + permission java.util.PropertyPermission "*", "read,write"; }; From ca2e0dc351ea2e19212007dd1d472610b2247b25 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Wed, 4 Oct 2023 08:26:13 +0530 Subject: [PATCH 12/19] Add unit tests Signed-off-by: Gagan Juneja --- .../telemetry/OTelTelemetryPlugin.java | 5 +++- .../metrics/OTelMetricsTelemetry.java | 14 ++++++---- .../tracing/OTelTracingTelemetry.java | 15 ++++++---- .../telemetry/OTelTelemetryPluginTests.java | 1 + .../metrics/OTelMetricsTelemetryTests.java | 24 +++++++++++----- .../tracing/OTelTracingTelemetryTests.java | 28 ++++++++++++------- 6 files changed, 57 insertions(+), 30 deletions(-) diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java index e8fc3fe53b1ef..479b0088879cc 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java @@ -68,7 +68,10 @@ public String getName() { private Telemetry telemetry(TelemetrySettings telemetrySettings) { final OpenTelemetrySdk openTelemetry = OTelResourceProvider.get(telemetrySettings, settings); - return new OTelTelemetry(new OTelTracingTelemetry(openTelemetry), new OTelMetricsTelemetry(openTelemetry)); + return new OTelTelemetry( + new OTelTracingTelemetry(openTelemetry, () -> openTelemetry.getSdkTracerProvider().close()), + new OTelMetricsTelemetry(openTelemetry, () -> openTelemetry.getSdkMeterProvider().close()) + ); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java index 5538f5b3625e4..c0260de1489d9 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java @@ -12,29 +12,32 @@ import org.apache.logging.log4j.Logger; import org.opensearch.telemetry.OTelTelemetryPlugin; +import java.io.Closeable; import java.security.AccessController; import java.security.PrivilegedAction; +import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.metrics.DoubleCounter; import io.opentelemetry.api.metrics.DoubleUpDownCounter; import io.opentelemetry.api.metrics.Meter; -import io.opentelemetry.sdk.OpenTelemetrySdk; /** * OTel implementation for {@link MetricsTelemetry} */ public class OTelMetricsTelemetry implements MetricsTelemetry { private static final Logger logger = LogManager.getLogger(OTelMetricsTelemetry.class); - private final OpenTelemetrySdk openTelemetry; + private final OpenTelemetry openTelemetry; private final Meter otelMeter; + private final Closeable metricsProviderClosable; /** * Constructor. * @param openTelemetry telemetry. */ - public OTelMetricsTelemetry(OpenTelemetrySdk openTelemetry) { + public OTelMetricsTelemetry(OpenTelemetry openTelemetry, Closeable metricsProviderClosable) { this.openTelemetry = openTelemetry; this.otelMeter = openTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); + this.metricsProviderClosable = metricsProviderClosable; } @Override @@ -63,11 +66,10 @@ public Counter createUpDownCounter(String name, String description, String unit) @Override public void close() { - // There is no harm closing the openTelemetry multiple times. try { - openTelemetry.getSdkMeterProvider().close(); + metricsProviderClosable.close(); } catch (Exception e) { - logger.warn("Error while closing Opentelemetry", e); + logger.warn("Error while closing Opentelemetry MeterProvider", e); } } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index ea7bce5080e76..00a3736c957ca 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -13,8 +13,10 @@ import org.opensearch.telemetry.OTelAttributesConverter; import org.opensearch.telemetry.OTelTelemetryPlugin; +import java.io.Closeable; + +import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.context.Context; -import io.opentelemetry.sdk.OpenTelemetrySdk; /** * OTel based Telemetry provider @@ -22,25 +24,26 @@ public class OTelTracingTelemetry implements TracingTelemetry { private static final Logger logger = LogManager.getLogger(OTelTracingTelemetry.class); - private final OpenTelemetrySdk openTelemetry; + private final OpenTelemetry openTelemetry; + private final Closeable tracerProviderClosable; private final io.opentelemetry.api.trace.Tracer otelTracer; /** * Creates OTel based Telemetry * @param openTelemetry OpenTelemetry instance */ - public OTelTracingTelemetry(OpenTelemetrySdk openTelemetry) { + public OTelTracingTelemetry(OpenTelemetry openTelemetry, Closeable tracerProviderClosable) { this.openTelemetry = openTelemetry; this.otelTracer = openTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); - + this.tracerProviderClosable = tracerProviderClosable; } @Override public void close() { try { - openTelemetry.getSdkTracerProvider().close(); + tracerProviderClosable.close(); } catch (Exception e) { - logger.warn("Error while closing Opentelemetry", e); + logger.warn("Error while closing Opentelemetry TracerProvider", e); } } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java index 2a6b9d687d2d9..967d2e8495d81 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java @@ -78,5 +78,6 @@ public void testGetTelemetry() { @After public void cleanup() { tracingTelemetry.close(); + metricsTelemetry.close(); } } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java index cf398e3467401..fcdb5f3609c6d 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java @@ -13,6 +13,9 @@ import org.opensearch.telemetry.metrics.tags.Tags; import org.opensearch.test.OpenSearchTestCase; +import java.util.concurrent.atomic.AtomicBoolean; + +import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.metrics.DoubleCounter; import io.opentelemetry.api.metrics.DoubleCounterBuilder; import io.opentelemetry.api.metrics.DoubleUpDownCounter; @@ -20,7 +23,6 @@ import io.opentelemetry.api.metrics.LongCounterBuilder; import io.opentelemetry.api.metrics.LongUpDownCounterBuilder; import io.opentelemetry.api.metrics.Meter; -import io.opentelemetry.sdk.OpenTelemetrySdk; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -32,14 +34,14 @@ public void testCounter() { String counterName = "test-counter"; String description = "test"; String unit = "1"; - OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); + OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Meter mockMeter = mock(Meter.class); DoubleCounter mockOTelDoubleCounter = mock(DoubleCounter.class); LongCounterBuilder mockOTelLongCounterBuilder = mock(LongCounterBuilder.class); DoubleCounterBuilder mockOTelDoubleCounterBuilder = mock(DoubleCounterBuilder.class); when(mockOpenTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); - MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry, () -> {}); when(mockMeter.counterBuilder(counterName)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setDescription(description)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setUnit(unit)).thenReturn(mockOTelLongCounterBuilder); @@ -58,14 +60,14 @@ public void testCounterNegativeValue() { String counterName = "test-counter"; String description = "test"; String unit = "1"; - OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); + OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Meter mockMeter = mock(Meter.class); DoubleCounter mockOTelDoubleCounter = mock(DoubleCounter.class); LongCounterBuilder mockOTelLongCounterBuilder = mock(LongCounterBuilder.class); DoubleCounterBuilder mockOTelDoubleCounterBuilder = mock(DoubleCounterBuilder.class); when(mockOpenTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); - MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry, () -> {}); when(mockMeter.counterBuilder(counterName)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setDescription(description)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setUnit(unit)).thenReturn(mockOTelLongCounterBuilder); @@ -81,14 +83,14 @@ public void testUpDownCounter() { String counterName = "test-counter"; String description = "test"; String unit = "1"; - OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); + OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Meter mockMeter = mock(Meter.class); DoubleUpDownCounter mockOTelUpDownDoubleCounter = mock(DoubleUpDownCounter.class); LongUpDownCounterBuilder mockOTelLongUpDownCounterBuilder = mock(LongUpDownCounterBuilder.class); DoubleUpDownCounterBuilder mockOTelDoubleUpDownCounterBuilder = mock(DoubleUpDownCounterBuilder.class); when(mockOpenTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); - MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry, () -> {}); when(mockMeter.upDownCounterBuilder(counterName)).thenReturn(mockOTelLongUpDownCounterBuilder); when(mockOTelLongUpDownCounterBuilder.setDescription(description)).thenReturn(mockOTelLongUpDownCounterBuilder); when(mockOTelLongUpDownCounterBuilder.setUnit(unit)).thenReturn(mockOTelLongUpDownCounterBuilder); @@ -102,4 +104,12 @@ public void testUpDownCounter() { counter.add(-2.0, tags); verify(mockOTelUpDownDoubleCounter).add((-2.0), OTelAttributesConverter.convert(tags)); } + + public void testClose() { + OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + final AtomicBoolean closed = new AtomicBoolean(false); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry, () -> closed.set(true)); + metricsTelemetry.close(); + assertTrue(closed.get()); + } } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java index d9d66f4ab4098..8bf1f2d8a1805 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java @@ -14,11 +14,12 @@ import java.util.Collections; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.api.trace.Tracer; -import io.opentelemetry.sdk.OpenTelemetrySdk; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -27,9 +28,8 @@ import static org.mockito.Mockito.when; public class OTelTracingTelemetryTests extends OpenSearchTestCase { - public void testCreateSpanWithoutParent() { - OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); + OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); @@ -39,7 +39,7 @@ public void testCreateSpanWithoutParent() { when(mockSpanBuilder.setSpanKind(any(io.opentelemetry.api.trace.SpanKind.class))).thenReturn(mockSpanBuilder); Map attributeMap = Collections.singletonMap("name", "value"); Attributes attributes = Attributes.create().addAttribute("name", "value"); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); Span span = tracingTelemetry.createSpan(SpanCreationContext.internal().name("span_name").attributes(attributes), null); verify(mockSpanBuilder, never()).setParent(any()); verify(mockSpanBuilder).setAllAttributes(createAttribute(attributes)); @@ -47,7 +47,7 @@ public void testCreateSpanWithoutParent() { } public void testCreateSpanWithParent() { - OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); + OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); @@ -59,7 +59,7 @@ public void testCreateSpanWithParent() { Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); Attributes attributes = Attributes.create().addAttribute("name", 1l); Span span = tracingTelemetry.createSpan(SpanCreationContext.internal().name("span_name").attributes(attributes), parentSpan); @@ -71,7 +71,7 @@ public void testCreateSpanWithParent() { } public void testCreateSpanWithParentWithMultipleAttributes() { - OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); + OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); @@ -83,7 +83,7 @@ public void testCreateSpanWithParentWithMultipleAttributes() { Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); Attributes attributes = Attributes.create() .addAttribute("key1", 1l) .addAttribute("key2", 2.0) @@ -117,13 +117,21 @@ private io.opentelemetry.api.common.Attributes createAttributeLong(Attributes at } public void testGetContextPropagator() { - OpenTelemetrySdk mockOpenTelemetry = mock(OpenTelemetrySdk.class); + OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); assertTrue(tracingTelemetry.getContextPropagator() instanceof OTelTracingContextPropagator); } + public void testClose() { + OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + final AtomicBoolean closed = new AtomicBoolean(false); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> closed.set(true)); + tracingTelemetry.close(); + assertTrue(closed.get()); + } + } From 692dfc485269de92292fb25c76c45689dba519ee Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Wed, 4 Oct 2023 10:42:02 +0530 Subject: [PATCH 13/19] Fix java doc Signed-off-by: Gagan Juneja --- .../telemetry/metrics/OTelMetricsTelemetry.java | 12 +++++++++--- .../telemetry/tracing/OTelTracingTelemetry.java | 7 ++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java index c0260de1489d9..09149a4259f28 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java @@ -31,13 +31,19 @@ public class OTelMetricsTelemetry implements MetricsTelemetry { private final Closeable metricsProviderClosable; /** - * Constructor. + * Creates OTel based {@link MetricsTelemetry}. * @param openTelemetry telemetry. */ - public OTelMetricsTelemetry(OpenTelemetry openTelemetry, Closeable metricsProviderClosable) { + + /** + * Creates OTel based {@link MetricsTelemetry}. + * @param openTelemetry OpenTelemetry instance + * @param meterProviderCloseable closable to close the meter. + */ + public OTelMetricsTelemetry(OpenTelemetry openTelemetry, Closeable meterProviderCloseable) { this.openTelemetry = openTelemetry; this.otelMeter = openTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); - this.metricsProviderClosable = metricsProviderClosable; + this.metricsProviderClosable = meterProviderCloseable; } @Override diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index 00a3736c957ca..2648222bed8d7 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -29,13 +29,14 @@ public class OTelTracingTelemetry implements TracingTelemetry { private final io.opentelemetry.api.trace.Tracer otelTracer; /** - * Creates OTel based Telemetry + * Creates OTel based {@link TracingTelemetry} * @param openTelemetry OpenTelemetry instance + * @param tracerProviderCloseable closable to close the tracer */ - public OTelTracingTelemetry(OpenTelemetry openTelemetry, Closeable tracerProviderClosable) { + public OTelTracingTelemetry(OpenTelemetry openTelemetry, Closeable tracerProviderCloseable) { this.openTelemetry = openTelemetry; this.otelTracer = openTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); - this.tracerProviderClosable = tracerProviderClosable; + this.tracerProviderClosable = tracerProviderCloseable; } @Override From 0373cfcb6e56c24579ee5d18b2727a4d982831f8 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Wed, 4 Oct 2023 10:42:59 +0530 Subject: [PATCH 14/19] Fix java doc Signed-off-by: Gagan Juneja --- .../opensearch/telemetry/metrics/OTelMetricsTelemetry.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java index 09149a4259f28..a9d05e3de3ab4 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java @@ -30,11 +30,6 @@ public class OTelMetricsTelemetry implements MetricsTelemetry { private final Meter otelMeter; private final Closeable metricsProviderClosable; - /** - * Creates OTel based {@link MetricsTelemetry}. - * @param openTelemetry telemetry. - */ - /** * Creates OTel based {@link MetricsTelemetry}. * @param openTelemetry OpenTelemetry instance From b20bcd2fe9c2fd08917c67f63128596fc08c1154 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Thu, 5 Oct 2023 01:19:47 +0530 Subject: [PATCH 15/19] Address review comment Signed-off-by: Gagan Juneja --- .../telemetry/OTelTelemetryPlugin.java | 13 +++++++-- .../metrics/OTelMetricsTelemetry.java | 12 ++++----- .../tracing/OTelTracingTelemetry.java | 12 +++++---- .../telemetry/OTelTelemetryPluginTests.java | 10 +++---- .../metrics/OTelMetricsTelemetryTests.java | 25 +++++++++-------- .../tracing/OTelTracingTelemetryTests.java | 27 ++++++++++--------- 6 files changed, 55 insertions(+), 44 deletions(-) diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java index 479b0088879cc..e7450e1662040 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java @@ -22,6 +22,8 @@ import java.util.Optional; import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.trace.SdkTracerProvider; /** * Telemetry plugin based on Otel @@ -69,8 +71,15 @@ public String getName() { private Telemetry telemetry(TelemetrySettings telemetrySettings) { final OpenTelemetrySdk openTelemetry = OTelResourceProvider.get(telemetrySettings, settings); return new OTelTelemetry( - new OTelTracingTelemetry(openTelemetry, () -> openTelemetry.getSdkTracerProvider().close()), - new OTelMetricsTelemetry(openTelemetry, () -> openTelemetry.getSdkMeterProvider().close()) + new OTelTracingTelemetry( + openTelemetry.getSdkTracerProvider(), + openTelemetry, + () -> openTelemetry.getSdkTracerProvider().close() + ), + new OTelMetricsTelemetry( + openTelemetry.getSdkMeterProvider(), + () -> openTelemetry.getSdkMeterProvider().close() + ) ); } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java index a9d05e3de3ab4..936f9b2640505 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java @@ -16,28 +16,26 @@ import java.security.AccessController; import java.security.PrivilegedAction; -import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.metrics.DoubleCounter; import io.opentelemetry.api.metrics.DoubleUpDownCounter; import io.opentelemetry.api.metrics.Meter; +import io.opentelemetry.api.metrics.MeterProvider; /** * OTel implementation for {@link MetricsTelemetry} */ -public class OTelMetricsTelemetry implements MetricsTelemetry { +public class OTelMetricsTelemetry implements MetricsTelemetry { private static final Logger logger = LogManager.getLogger(OTelMetricsTelemetry.class); - private final OpenTelemetry openTelemetry; private final Meter otelMeter; private final Closeable metricsProviderClosable; /** * Creates OTel based {@link MetricsTelemetry}. - * @param openTelemetry OpenTelemetry instance + * @param meterProvider OpenTelemetry instance * @param meterProviderCloseable closable to close the meter. */ - public OTelMetricsTelemetry(OpenTelemetry openTelemetry, Closeable meterProviderCloseable) { - this.openTelemetry = openTelemetry; - this.otelMeter = openTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); + public OTelMetricsTelemetry(T meterProvider, Closeable meterProviderCloseable) { + this.otelMeter = meterProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); this.metricsProviderClosable = meterProviderCloseable; } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index 2648222bed8d7..d0168bf6986d8 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -14,14 +14,16 @@ import org.opensearch.telemetry.OTelTelemetryPlugin; import java.io.Closeable; +import java.io.IOException; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.trace.TracerProvider; import io.opentelemetry.context.Context; /** * OTel based Telemetry provider */ -public class OTelTracingTelemetry implements TracingTelemetry { +public class OTelTracingTelemetry implements TracingTelemetry { private static final Logger logger = LogManager.getLogger(OTelTracingTelemetry.class); private final OpenTelemetry openTelemetry; @@ -30,12 +32,12 @@ public class OTelTracingTelemetry implements TracingTelemetry { /** * Creates OTel based {@link TracingTelemetry} - * @param openTelemetry OpenTelemetry instance + * @param tracerProvider OpenTelemetry instance * @param tracerProviderCloseable closable to close the tracer */ - public OTelTracingTelemetry(OpenTelemetry openTelemetry, Closeable tracerProviderCloseable) { + public OTelTracingTelemetry(T tracerProvider, OpenTelemetry openTelemetry, Closeable tracerProviderCloseable) { + this.otelTracer = tracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); this.openTelemetry = openTelemetry; - this.otelTracer = openTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); this.tracerProviderClosable = tracerProviderCloseable; } @@ -43,7 +45,7 @@ public OTelTracingTelemetry(OpenTelemetry openTelemetry, Closeable tracerProvide public void close() { try { tracerProviderClosable.close(); - } catch (Exception e) { + } catch (IOException e) { logger.warn("Error while closing Opentelemetry TracerProvider", e); } } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java index 967d2e8495d81..2d76d46478f2e 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java @@ -37,7 +37,7 @@ public class OTelTelemetryPluginTests extends OpenSearchTestCase { - private OTelTelemetryPlugin oTelTelemetryModulePlugin; + private OTelTelemetryPlugin oTelTelemetryPlugin; private Optional telemetry; private TracingTelemetry tracingTelemetry; @@ -48,8 +48,8 @@ public void setup() { // TRACER_EXPORTER_DELAY_SETTING should always be less than 10 seconds because // io.opentelemetry.sdk.OpenTelemetrySdk.close waits only for 10 seconds for shutdown to complete. Settings settings = Settings.builder().put(TRACER_EXPORTER_DELAY_SETTING.getKey(), "1s").build(); - oTelTelemetryModulePlugin = new OTelTelemetryPlugin(settings); - telemetry = oTelTelemetryModulePlugin.getTelemetry( + oTelTelemetryPlugin = new OTelTelemetryPlugin(settings); + telemetry = oTelTelemetryPlugin.getTelemetry( new TelemetrySettings(Settings.EMPTY, new ClusterSettings(settings, Set.of(TRACER_ENABLED_SETTING, TRACER_SAMPLER_PROBABILITY))) ); tracingTelemetry = telemetry.get().getTracingTelemetry(); @@ -59,7 +59,7 @@ public void setup() { public void testGetTelemetry() { Set> allTracerSettings = new HashSet<>(); ClusterSettings.FEATURE_FLAGGED_CLUSTER_SETTINGS.get(List.of(FeatureFlags.TELEMETRY)).stream().forEach((allTracerSettings::add)); - assertEquals(OTEL_TRACER_NAME, oTelTelemetryModulePlugin.getName()); + assertEquals(OTEL_TRACER_NAME, oTelTelemetryPlugin.getName()); assertTrue(tracingTelemetry instanceof OTelTracingTelemetry); assertTrue(metricsTelemetry instanceof OTelMetricsTelemetry); assertEquals( @@ -70,7 +70,7 @@ public void testGetTelemetry() { OTEL_TRACER_SPAN_EXPORTER_CLASS_SETTING, OTEL_METRICS_EXPORTER_CLASS_SETTING ), - oTelTelemetryModulePlugin.getSettings() + oTelTelemetryPlugin.getSettings() ); } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java index fcdb5f3609c6d..eed48e711f434 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java @@ -15,7 +15,6 @@ import java.util.concurrent.atomic.AtomicBoolean; -import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.metrics.DoubleCounter; import io.opentelemetry.api.metrics.DoubleCounterBuilder; import io.opentelemetry.api.metrics.DoubleUpDownCounter; @@ -23,6 +22,7 @@ import io.opentelemetry.api.metrics.LongCounterBuilder; import io.opentelemetry.api.metrics.LongUpDownCounterBuilder; import io.opentelemetry.api.metrics.Meter; +import io.opentelemetry.api.metrics.MeterProvider; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -34,14 +34,13 @@ public void testCounter() { String counterName = "test-counter"; String description = "test"; String unit = "1"; - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Meter mockMeter = mock(Meter.class); DoubleCounter mockOTelDoubleCounter = mock(DoubleCounter.class); LongCounterBuilder mockOTelLongCounterBuilder = mock(LongCounterBuilder.class); DoubleCounterBuilder mockOTelDoubleCounterBuilder = mock(DoubleCounterBuilder.class); - - when(mockOpenTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); - MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry, () -> {}); + MeterProvider meterProvider = mock(MeterProvider.class); + when(meterProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(meterProvider, () -> {}); when(mockMeter.counterBuilder(counterName)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setDescription(description)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setUnit(unit)).thenReturn(mockOTelLongCounterBuilder); @@ -60,14 +59,14 @@ public void testCounterNegativeValue() { String counterName = "test-counter"; String description = "test"; String unit = "1"; - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Meter mockMeter = mock(Meter.class); DoubleCounter mockOTelDoubleCounter = mock(DoubleCounter.class); LongCounterBuilder mockOTelLongCounterBuilder = mock(LongCounterBuilder.class); DoubleCounterBuilder mockOTelDoubleCounterBuilder = mock(DoubleCounterBuilder.class); - when(mockOpenTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); - MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry, () -> {}); + MeterProvider meterProvider = mock(MeterProvider.class); + when(meterProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(meterProvider, () -> {}); when(mockMeter.counterBuilder(counterName)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setDescription(description)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setUnit(unit)).thenReturn(mockOTelLongCounterBuilder); @@ -83,14 +82,14 @@ public void testUpDownCounter() { String counterName = "test-counter"; String description = "test"; String unit = "1"; - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Meter mockMeter = mock(Meter.class); DoubleUpDownCounter mockOTelUpDownDoubleCounter = mock(DoubleUpDownCounter.class); LongUpDownCounterBuilder mockOTelLongUpDownCounterBuilder = mock(LongUpDownCounterBuilder.class); DoubleUpDownCounterBuilder mockOTelDoubleUpDownCounterBuilder = mock(DoubleUpDownCounterBuilder.class); - when(mockOpenTelemetry.getMeter(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); - MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry, () -> {}); + MeterProvider meterProvider = mock(MeterProvider.class); + when(meterProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(meterProvider, () -> {}); when(mockMeter.upDownCounterBuilder(counterName)).thenReturn(mockOTelLongUpDownCounterBuilder); when(mockOTelLongUpDownCounterBuilder.setDescription(description)).thenReturn(mockOTelLongUpDownCounterBuilder); when(mockOTelLongUpDownCounterBuilder.setUnit(unit)).thenReturn(mockOTelLongUpDownCounterBuilder); @@ -106,9 +105,9 @@ public void testUpDownCounter() { } public void testClose() { - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); final AtomicBoolean closed = new AtomicBoolean(false); - MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(mockOpenTelemetry, () -> closed.set(true)); + MeterProvider meterProvider = mock(MeterProvider.class); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(meterProvider, () -> closed.set(true)); metricsTelemetry.close(); assertTrue(closed.get()); } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java index 8bf1f2d8a1805..37925ee8ccb87 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java @@ -12,14 +12,13 @@ import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.test.OpenSearchTestCase; -import java.util.Collections; -import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.api.trace.TracerProvider; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -30,16 +29,16 @@ public class OTelTracingTelemetryTests extends OpenSearchTestCase { public void testCreateSpanWithoutParent() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + TracerProvider tracerProvider = mock(TracerProvider.class); Tracer mockTracer = mock(Tracer.class); - when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); + when(tracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); when(mockSpanBuilder.setAllAttributes(any(io.opentelemetry.api.common.Attributes.class))).thenReturn(mockSpanBuilder); when(mockSpanBuilder.startSpan()).thenReturn(mock(io.opentelemetry.api.trace.Span.class)); when(mockSpanBuilder.setSpanKind(any(io.opentelemetry.api.trace.SpanKind.class))).thenReturn(mockSpanBuilder); - Map attributeMap = Collections.singletonMap("name", "value"); Attributes attributes = Attributes.create().addAttribute("name", "value"); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(tracerProvider, mockOpenTelemetry, () -> {}); Span span = tracingTelemetry.createSpan(SpanCreationContext.internal().name("span_name").attributes(attributes), null); verify(mockSpanBuilder, never()).setParent(any()); verify(mockSpanBuilder).setAllAttributes(createAttribute(attributes)); @@ -49,7 +48,8 @@ public void testCreateSpanWithoutParent() { public void testCreateSpanWithParent() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); + TracerProvider tracerProvider = mock(TracerProvider.class); + when(tracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); when(mockSpanBuilder.setParent(any())).thenReturn(mockSpanBuilder); @@ -59,7 +59,7 @@ public void testCreateSpanWithParent() { Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(tracerProvider, mockOpenTelemetry, () -> {}); Attributes attributes = Attributes.create().addAttribute("name", 1l); Span span = tracingTelemetry.createSpan(SpanCreationContext.internal().name("span_name").attributes(attributes), parentSpan); @@ -73,7 +73,8 @@ public void testCreateSpanWithParent() { public void testCreateSpanWithParentWithMultipleAttributes() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); + TracerProvider tracerProvider = mock(TracerProvider.class); + when(tracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); when(mockSpanBuilder.setParent(any())).thenReturn(mockSpanBuilder); @@ -83,7 +84,7 @@ public void testCreateSpanWithParentWithMultipleAttributes() { Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(tracerProvider, mockOpenTelemetry, () -> {}); Attributes attributes = Attributes.create() .addAttribute("key1", 1l) .addAttribute("key2", 2.0) @@ -119,17 +120,19 @@ private io.opentelemetry.api.common.Attributes createAttributeLong(Attributes at public void testGetContextPropagator() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); + TracerProvider tracerProvider = mock(TracerProvider.class); + when(tracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(tracerProvider, mockOpenTelemetry, () -> {}); assertTrue(tracingTelemetry.getContextPropagator() instanceof OTelTracingContextPropagator); } public void testClose() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); + TracerProvider tracerProvider = mock(TracerProvider.class); final AtomicBoolean closed = new AtomicBoolean(false); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> closed.set(true)); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(tracerProvider, mockOpenTelemetry, () -> closed.set(true)); tracingTelemetry.close(); assertTrue(closed.get()); } From 2ed5ace1c05bf5297df4ce3c06e87e51afbc6929 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Thu, 5 Oct 2023 01:27:45 +0530 Subject: [PATCH 16/19] Fix java doc Signed-off-by: Gagan Juneja --- .../opensearch/telemetry/metrics/OTelMetricsTelemetry.java | 5 +++-- .../opensearch/telemetry/tracing/OTelTracingTelemetry.java | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java index 936f9b2640505..49c6689037773 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java @@ -13,6 +13,7 @@ import org.opensearch.telemetry.OTelTelemetryPlugin; import java.io.Closeable; +import java.io.IOException; import java.security.AccessController; import java.security.PrivilegedAction; @@ -31,7 +32,7 @@ public class OTelMetricsTelemetry implement /** * Creates OTel based {@link MetricsTelemetry}. - * @param meterProvider OpenTelemetry instance + * @param meterProvider {@link MeterProvider} instance * @param meterProviderCloseable closable to close the meter. */ public OTelMetricsTelemetry(T meterProvider, Closeable meterProviderCloseable) { @@ -67,7 +68,7 @@ public Counter createUpDownCounter(String name, String description, String unit) public void close() { try { metricsProviderClosable.close(); - } catch (Exception e) { + } catch (IOException e) { logger.warn("Error while closing Opentelemetry MeterProvider", e); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index d0168bf6986d8..a4f0638dfd46e 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -32,7 +32,8 @@ public class OTelTracingTelemetry implemen /** * Creates OTel based {@link TracingTelemetry} - * @param tracerProvider OpenTelemetry instance + * @param tracerProvider {@link TracerProvider} instance. + * @param openTelemetry OpenTelemetry instance * @param tracerProviderCloseable closable to close the tracer */ public OTelTracingTelemetry(T tracerProvider, OpenTelemetry openTelemetry, Closeable tracerProviderCloseable) { From eef4d9ac696c449b2ad3fd385c1e940653d53724 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Thu, 5 Oct 2023 01:55:34 +0530 Subject: [PATCH 17/19] Address review comment Signed-off-by: Gagan Juneja --- .../telemetry/metrics/MetricsTelemetry.java | 5 +--- .../telemetry/tracing/TracingTelemetry.java | 5 ---- .../telemetry/OTelTelemetryPlugin.java | 7 +---- .../metrics/OTelMetricsTelemetry.java | 8 ++---- .../tracing/OTelTracingTelemetry.java | 18 +++++-------- .../telemetry/OTelTelemetryPluginTests.java | 3 ++- .../metrics/OTelMetricsTelemetryTests.java | 3 ++- .../tracing/OTelTracingTelemetryTests.java | 27 ++++++++----------- 8 files changed, 25 insertions(+), 51 deletions(-) diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java index f310558ab0124..2f70c28efb1cd 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/metrics/MetricsTelemetry.java @@ -19,8 +19,5 @@ */ @ExperimentalApi public interface MetricsTelemetry extends MetricsRegistry, Closeable { - /** - * closes the resource - */ - void close(); + } diff --git a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java index 38c9104c3ec35..f04a505088424 100644 --- a/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java +++ b/libs/telemetry/src/main/java/org/opensearch/telemetry/tracing/TracingTelemetry.java @@ -35,9 +35,4 @@ public interface TracingTelemetry extends Closeable { */ TracingContextPropagator getContextPropagator(); - /** - * closes the resource - */ - void close(); - } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java index e7450e1662040..9f824bc2b6c19 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java @@ -23,7 +23,6 @@ import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.metrics.SdkMeterProvider; -import io.opentelemetry.sdk.trace.SdkTracerProvider; /** * Telemetry plugin based on Otel @@ -71,11 +70,7 @@ public String getName() { private Telemetry telemetry(TelemetrySettings telemetrySettings) { final OpenTelemetrySdk openTelemetry = OTelResourceProvider.get(telemetrySettings, settings); return new OTelTelemetry( - new OTelTracingTelemetry( - openTelemetry.getSdkTracerProvider(), - openTelemetry, - () -> openTelemetry.getSdkTracerProvider().close() - ), + new OTelTracingTelemetry(openTelemetry, () -> openTelemetry.getSdkTracerProvider().close()), new OTelMetricsTelemetry( openTelemetry.getSdkMeterProvider(), () -> openTelemetry.getSdkMeterProvider().close() diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java index 49c6689037773..1ecfabceb3500 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java @@ -65,11 +65,7 @@ public Counter createUpDownCounter(String name, String description, String unit) } @Override - public void close() { - try { - metricsProviderClosable.close(); - } catch (IOException e) { - logger.warn("Error while closing Opentelemetry MeterProvider", e); - } + public void close() throws IOException { + metricsProviderClosable.close(); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index a4f0638dfd46e..0c89cb37e681c 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -17,38 +17,32 @@ import java.io.IOException; import io.opentelemetry.api.OpenTelemetry; -import io.opentelemetry.api.trace.TracerProvider; import io.opentelemetry.context.Context; /** * OTel based Telemetry provider */ -public class OTelTracingTelemetry implements TracingTelemetry { +public class OTelTracingTelemetry implements TracingTelemetry { private static final Logger logger = LogManager.getLogger(OTelTracingTelemetry.class); - private final OpenTelemetry openTelemetry; + private final T openTelemetry; private final Closeable tracerProviderClosable; private final io.opentelemetry.api.trace.Tracer otelTracer; /** * Creates OTel based {@link TracingTelemetry} - * @param tracerProvider {@link TracerProvider} instance. * @param openTelemetry OpenTelemetry instance * @param tracerProviderCloseable closable to close the tracer */ - public OTelTracingTelemetry(T tracerProvider, OpenTelemetry openTelemetry, Closeable tracerProviderCloseable) { - this.otelTracer = tracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); + public OTelTracingTelemetry(T openTelemetry, Closeable tracerProviderCloseable) { this.openTelemetry = openTelemetry; + this.otelTracer = openTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); this.tracerProviderClosable = tracerProviderCloseable; } @Override - public void close() { - try { - tracerProviderClosable.close(); - } catch (IOException e) { - logger.warn("Error while closing Opentelemetry TracerProvider", e); - } + public void close() throws IOException { + tracerProviderClosable.close(); } @Override diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java index 2d76d46478f2e..2fcf89947e537 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/OTelTelemetryPluginTests.java @@ -20,6 +20,7 @@ import org.junit.After; import org.junit.Before; +import java.io.IOException; import java.util.Arrays; import java.util.HashSet; import java.util.List; @@ -76,7 +77,7 @@ public void testGetTelemetry() { } @After - public void cleanup() { + public void cleanup() throws IOException { tracingTelemetry.close(); metricsTelemetry.close(); } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java index eed48e711f434..ff3f2583620d8 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java @@ -13,6 +13,7 @@ import org.opensearch.telemetry.metrics.tags.Tags; import org.opensearch.test.OpenSearchTestCase; +import java.io.IOException; import java.util.concurrent.atomic.AtomicBoolean; import io.opentelemetry.api.metrics.DoubleCounter; @@ -104,7 +105,7 @@ public void testUpDownCounter() { verify(mockOTelUpDownDoubleCounter).add((-2.0), OTelAttributesConverter.convert(tags)); } - public void testClose() { + public void testClose() throws IOException { final AtomicBoolean closed = new AtomicBoolean(false); MeterProvider meterProvider = mock(MeterProvider.class); MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(meterProvider, () -> closed.set(true)); diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java index 37925ee8ccb87..5e454d3eb6d77 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java @@ -12,13 +12,13 @@ import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.test.OpenSearchTestCase; +import java.io.IOException; import java.util.concurrent.atomic.AtomicBoolean; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.api.trace.Tracer; -import io.opentelemetry.api.trace.TracerProvider; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -29,16 +29,15 @@ public class OTelTracingTelemetryTests extends OpenSearchTestCase { public void testCreateSpanWithoutParent() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); - TracerProvider tracerProvider = mock(TracerProvider.class); Tracer mockTracer = mock(Tracer.class); - when(tracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); + when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); when(mockSpanBuilder.setAllAttributes(any(io.opentelemetry.api.common.Attributes.class))).thenReturn(mockSpanBuilder); when(mockSpanBuilder.startSpan()).thenReturn(mock(io.opentelemetry.api.trace.Span.class)); when(mockSpanBuilder.setSpanKind(any(io.opentelemetry.api.trace.SpanKind.class))).thenReturn(mockSpanBuilder); Attributes attributes = Attributes.create().addAttribute("name", "value"); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(tracerProvider, mockOpenTelemetry, () -> {}); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); Span span = tracingTelemetry.createSpan(SpanCreationContext.internal().name("span_name").attributes(attributes), null); verify(mockSpanBuilder, never()).setParent(any()); verify(mockSpanBuilder).setAllAttributes(createAttribute(attributes)); @@ -48,8 +47,7 @@ public void testCreateSpanWithoutParent() { public void testCreateSpanWithParent() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - TracerProvider tracerProvider = mock(TracerProvider.class); - when(tracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); + when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); when(mockSpanBuilder.setParent(any())).thenReturn(mockSpanBuilder); @@ -59,7 +57,7 @@ public void testCreateSpanWithParent() { Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(tracerProvider, mockOpenTelemetry, () -> {}); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); Attributes attributes = Attributes.create().addAttribute("name", 1l); Span span = tracingTelemetry.createSpan(SpanCreationContext.internal().name("span_name").attributes(attributes), parentSpan); @@ -73,8 +71,7 @@ public void testCreateSpanWithParent() { public void testCreateSpanWithParentWithMultipleAttributes() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - TracerProvider tracerProvider = mock(TracerProvider.class); - when(tracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); + when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); when(mockSpanBuilder.setParent(any())).thenReturn(mockSpanBuilder); @@ -84,7 +81,7 @@ public void testCreateSpanWithParentWithMultipleAttributes() { Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(tracerProvider, mockOpenTelemetry, () -> {}); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); Attributes attributes = Attributes.create() .addAttribute("key1", 1l) .addAttribute("key2", 2.0) @@ -120,19 +117,17 @@ private io.opentelemetry.api.common.Attributes createAttributeLong(Attributes at public void testGetContextPropagator() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - TracerProvider tracerProvider = mock(TracerProvider.class); - when(tracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); + when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(tracerProvider, mockOpenTelemetry, () -> {}); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); assertTrue(tracingTelemetry.getContextPropagator() instanceof OTelTracingContextPropagator); } - public void testClose() { + public void testClose() throws IOException { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); - TracerProvider tracerProvider = mock(TracerProvider.class); final AtomicBoolean closed = new AtomicBoolean(false); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(tracerProvider, mockOpenTelemetry, () -> closed.set(true)); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> closed.set(true)); tracingTelemetry.close(); assertTrue(closed.get()); } From 890b48badf05fc965b62095b52d56999a0cb7127 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Thu, 5 Oct 2023 02:23:37 +0530 Subject: [PATCH 18/19] Address review comment Signed-off-by: Gagan Juneja --- .../telemetry/OTelTelemetryPlugin.java | 8 ++--- .../metrics/OTelMetricsTelemetry.java | 9 +++-- .../tracing/OTelTracingTelemetry.java | 17 ++++----- .../metrics/OTelMetricsTelemetryTests.java | 20 ++++------- .../tracing/OTelTracingTelemetryTests.java | 36 +++++++++---------- 5 files changed, 38 insertions(+), 52 deletions(-) diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java index 9f824bc2b6c19..b57876c9310f3 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/OTelTelemetryPlugin.java @@ -22,7 +22,6 @@ import java.util.Optional; import io.opentelemetry.sdk.OpenTelemetrySdk; -import io.opentelemetry.sdk.metrics.SdkMeterProvider; /** * Telemetry plugin based on Otel @@ -70,11 +69,8 @@ public String getName() { private Telemetry telemetry(TelemetrySettings telemetrySettings) { final OpenTelemetrySdk openTelemetry = OTelResourceProvider.get(telemetrySettings, settings); return new OTelTelemetry( - new OTelTracingTelemetry(openTelemetry, () -> openTelemetry.getSdkTracerProvider().close()), - new OTelMetricsTelemetry( - openTelemetry.getSdkMeterProvider(), - () -> openTelemetry.getSdkMeterProvider().close() - ) + new OTelTracingTelemetry<>(openTelemetry, openTelemetry.getSdkTracerProvider()), + new OTelMetricsTelemetry<>(openTelemetry.getSdkMeterProvider()) ); } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java index 1ecfabceb3500..78fa5647ebc2e 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java @@ -28,16 +28,15 @@ public class OTelMetricsTelemetry implements MetricsTelemetry { private static final Logger logger = LogManager.getLogger(OTelMetricsTelemetry.class); private final Meter otelMeter; - private final Closeable metricsProviderClosable; + private final T meterProvider; /** * Creates OTel based {@link MetricsTelemetry}. * @param meterProvider {@link MeterProvider} instance - * @param meterProviderCloseable closable to close the meter. */ - public OTelMetricsTelemetry(T meterProvider, Closeable meterProviderCloseable) { + public OTelMetricsTelemetry(T meterProvider) { + this.meterProvider = meterProvider; this.otelMeter = meterProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); - this.metricsProviderClosable = meterProviderCloseable; } @Override @@ -66,6 +65,6 @@ public Counter createUpDownCounter(String name, String description, String unit) @Override public void close() throws IOException { - metricsProviderClosable.close(); + meterProvider.close(); } } diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index 0c89cb37e681c..9e8065664b825 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -17,32 +17,33 @@ import java.io.IOException; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.trace.TracerProvider; import io.opentelemetry.context.Context; /** * OTel based Telemetry provider */ -public class OTelTracingTelemetry implements TracingTelemetry { +public class OTelTracingTelemetry implements TracingTelemetry { private static final Logger logger = LogManager.getLogger(OTelTracingTelemetry.class); - private final T openTelemetry; - private final Closeable tracerProviderClosable; + private final OpenTelemetry openTelemetry; + private final T tracerProvider; private final io.opentelemetry.api.trace.Tracer otelTracer; /** * Creates OTel based {@link TracingTelemetry} * @param openTelemetry OpenTelemetry instance - * @param tracerProviderCloseable closable to close the tracer + * @param tracerProvider {@link TracerProvider} instance. */ - public OTelTracingTelemetry(T openTelemetry, Closeable tracerProviderCloseable) { + public OTelTracingTelemetry(OpenTelemetry openTelemetry, T tracerProvider) { this.openTelemetry = openTelemetry; - this.otelTracer = openTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); - this.tracerProviderClosable = tracerProviderCloseable; + this.tracerProvider = tracerProvider; + this.otelTracer = tracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME); } @Override public void close() throws IOException { - tracerProviderClosable.close(); + tracerProvider.close(); } @Override diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java index ff3f2583620d8..233c93e6b9a36 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetryTests.java @@ -13,9 +13,6 @@ import org.opensearch.telemetry.metrics.tags.Tags; import org.opensearch.test.OpenSearchTestCase; -import java.io.IOException; -import java.util.concurrent.atomic.AtomicBoolean; - import io.opentelemetry.api.metrics.DoubleCounter; import io.opentelemetry.api.metrics.DoubleCounterBuilder; import io.opentelemetry.api.metrics.DoubleUpDownCounter; @@ -31,6 +28,7 @@ public class OTelMetricsTelemetryTests extends OpenSearchTestCase { + @SuppressWarnings({ "rawtypes", "unchecked" }) public void testCounter() { String counterName = "test-counter"; String description = "test"; @@ -41,7 +39,7 @@ public void testCounter() { DoubleCounterBuilder mockOTelDoubleCounterBuilder = mock(DoubleCounterBuilder.class); MeterProvider meterProvider = mock(MeterProvider.class); when(meterProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); - MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(meterProvider, () -> {}); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(meterProvider); when(mockMeter.counterBuilder(counterName)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setDescription(description)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setUnit(unit)).thenReturn(mockOTelLongCounterBuilder); @@ -56,6 +54,7 @@ public void testCounter() { verify(mockOTelDoubleCounter).add(2.0, OTelAttributesConverter.convert(tags)); } + @SuppressWarnings({ "rawtypes", "unchecked" }) public void testCounterNegativeValue() { String counterName = "test-counter"; String description = "test"; @@ -67,7 +66,7 @@ public void testCounterNegativeValue() { MeterProvider meterProvider = mock(MeterProvider.class); when(meterProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); - MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(meterProvider, () -> {}); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(meterProvider); when(mockMeter.counterBuilder(counterName)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setDescription(description)).thenReturn(mockOTelLongCounterBuilder); when(mockOTelLongCounterBuilder.setUnit(unit)).thenReturn(mockOTelLongCounterBuilder); @@ -79,6 +78,7 @@ public void testCounterNegativeValue() { verify(mockOTelDoubleCounter).add(-1.0); } + @SuppressWarnings({ "rawtypes", "unchecked" }) public void testUpDownCounter() { String counterName = "test-counter"; String description = "test"; @@ -90,7 +90,7 @@ public void testUpDownCounter() { MeterProvider meterProvider = mock(MeterProvider.class); when(meterProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockMeter); - MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(meterProvider, () -> {}); + MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(meterProvider); when(mockMeter.upDownCounterBuilder(counterName)).thenReturn(mockOTelLongUpDownCounterBuilder); when(mockOTelLongUpDownCounterBuilder.setDescription(description)).thenReturn(mockOTelLongUpDownCounterBuilder); when(mockOTelLongUpDownCounterBuilder.setUnit(unit)).thenReturn(mockOTelLongUpDownCounterBuilder); @@ -104,12 +104,4 @@ public void testUpDownCounter() { counter.add(-2.0, tags); verify(mockOTelUpDownDoubleCounter).add((-2.0), OTelAttributesConverter.convert(tags)); } - - public void testClose() throws IOException { - final AtomicBoolean closed = new AtomicBoolean(false); - MeterProvider meterProvider = mock(MeterProvider.class); - MetricsTelemetry metricsTelemetry = new OTelMetricsTelemetry(meterProvider, () -> closed.set(true)); - metricsTelemetry.close(); - assertTrue(closed.get()); - } } diff --git a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java index 5e454d3eb6d77..1a508ed252493 100644 --- a/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java +++ b/plugins/telemetry-otel/src/test/java/org/opensearch/telemetry/tracing/OTelTracingTelemetryTests.java @@ -12,13 +12,11 @@ import org.opensearch.telemetry.tracing.attributes.Attributes; import org.opensearch.test.OpenSearchTestCase; -import java.io.IOException; -import java.util.concurrent.atomic.AtomicBoolean; - import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.api.trace.SpanBuilder; import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.api.trace.TracerProvider; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -27,27 +25,31 @@ import static org.mockito.Mockito.when; public class OTelTracingTelemetryTests extends OpenSearchTestCase { + @SuppressWarnings({ "rawtypes", "unchecked" }) public void testCreateSpanWithoutParent() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); + TracerProvider mockTracerProvider = mock(TracerProvider.class); + when(mockTracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); when(mockSpanBuilder.setAllAttributes(any(io.opentelemetry.api.common.Attributes.class))).thenReturn(mockSpanBuilder); when(mockSpanBuilder.startSpan()).thenReturn(mock(io.opentelemetry.api.trace.Span.class)); when(mockSpanBuilder.setSpanKind(any(io.opentelemetry.api.trace.SpanKind.class))).thenReturn(mockSpanBuilder); Attributes attributes = Attributes.create().addAttribute("name", "value"); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, mockTracerProvider); Span span = tracingTelemetry.createSpan(SpanCreationContext.internal().name("span_name").attributes(attributes), null); verify(mockSpanBuilder, never()).setParent(any()); verify(mockSpanBuilder).setAllAttributes(createAttribute(attributes)); assertNull(span.getParentSpan()); } + @SuppressWarnings({ "rawtypes", "unchecked" }) public void testCreateSpanWithParent() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); + TracerProvider mockTracerProvider = mock(TracerProvider.class); + when(mockTracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); when(mockSpanBuilder.setParent(any())).thenReturn(mockSpanBuilder); @@ -57,7 +59,7 @@ public void testCreateSpanWithParent() { Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, mockTracerProvider); Attributes attributes = Attributes.create().addAttribute("name", 1l); Span span = tracingTelemetry.createSpan(SpanCreationContext.internal().name("span_name").attributes(attributes), parentSpan); @@ -68,10 +70,12 @@ public void testCreateSpanWithParent() { assertEquals("parent_span", span.getParentSpan().getSpanName()); } + @SuppressWarnings({ "rawtypes", "unchecked" }) public void testCreateSpanWithParentWithMultipleAttributes() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); + TracerProvider mockTracerProvider = mock(TracerProvider.class); + when(mockTracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); SpanBuilder mockSpanBuilder = mock(SpanBuilder.class); when(mockTracer.spanBuilder("span_name")).thenReturn(mockSpanBuilder); when(mockSpanBuilder.setParent(any())).thenReturn(mockSpanBuilder); @@ -81,7 +85,7 @@ public void testCreateSpanWithParentWithMultipleAttributes() { Span parentSpan = new OTelSpan("parent_span", mock(io.opentelemetry.api.trace.Span.class), null); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, mockTracerProvider); Attributes attributes = Attributes.create() .addAttribute("key1", 1l) .addAttribute("key2", 2.0) @@ -114,22 +118,16 @@ private io.opentelemetry.api.common.Attributes createAttributeLong(Attributes at return attributesBuilder.build(); } + @SuppressWarnings({ "rawtypes", "unchecked" }) public void testGetContextPropagator() { OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); Tracer mockTracer = mock(Tracer.class); - when(mockOpenTelemetry.getTracer(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); + TracerProvider mockTracerProvider = mock(TracerProvider.class); + when(mockTracerProvider.get(OTelTelemetryPlugin.INSTRUMENTATION_SCOPE_NAME)).thenReturn(mockTracer); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> {}); + TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, mockTracerProvider); assertTrue(tracingTelemetry.getContextPropagator() instanceof OTelTracingContextPropagator); } - public void testClose() throws IOException { - OpenTelemetry mockOpenTelemetry = mock(OpenTelemetry.class); - final AtomicBoolean closed = new AtomicBoolean(false); - TracingTelemetry tracingTelemetry = new OTelTracingTelemetry(mockOpenTelemetry, () -> closed.set(true)); - tracingTelemetry.close(); - assertTrue(closed.get()); - } - } From 7ccb6697d122c97c3cc9f74af390bf07318293b3 Mon Sep 17 00:00:00 2001 From: Gagan Juneja Date: Thu, 5 Oct 2023 02:32:37 +0530 Subject: [PATCH 19/19] Address review comment Signed-off-by: Gagan Juneja --- .../opensearch/telemetry/metrics/OTelMetricsTelemetry.java | 3 --- .../opensearch/telemetry/tracing/OTelTracingTelemetry.java | 4 ---- 2 files changed, 7 deletions(-) diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java index 78fa5647ebc2e..8598e5976d20d 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/metrics/OTelMetricsTelemetry.java @@ -8,8 +8,6 @@ package org.opensearch.telemetry.metrics; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.opensearch.telemetry.OTelTelemetryPlugin; import java.io.Closeable; @@ -26,7 +24,6 @@ * OTel implementation for {@link MetricsTelemetry} */ public class OTelMetricsTelemetry implements MetricsTelemetry { - private static final Logger logger = LogManager.getLogger(OTelMetricsTelemetry.class); private final Meter otelMeter; private final T meterProvider; diff --git a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java index 9e8065664b825..f88afe623fd56 100644 --- a/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java +++ b/plugins/telemetry-otel/src/main/java/org/opensearch/telemetry/tracing/OTelTracingTelemetry.java @@ -8,8 +8,6 @@ package org.opensearch.telemetry.tracing; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import org.opensearch.telemetry.OTelAttributesConverter; import org.opensearch.telemetry.OTelTelemetryPlugin; @@ -24,8 +22,6 @@ * OTel based Telemetry provider */ public class OTelTracingTelemetry implements TracingTelemetry { - - private static final Logger logger = LogManager.getLogger(OTelTracingTelemetry.class); private final OpenTelemetry openTelemetry; private final T tracerProvider; private final io.opentelemetry.api.trace.Tracer otelTracer;