Centralize OpenTelemetry Constants and Helpers in source/extensions/common/opentelemetry by mmorel-35 · Pull Request #41011 · envoyproxy/envoy

mmorel-35 · 2025-09-07T12:18:53Z

Commit Message:
Centralizes OpenTelemetry-related constants, helpers, and types into source/extensions/common/opentelemetry as described in #41010.

Additional Description:

Moves duplicated OpenTelemetry constants and utility functions from tracing and propagator extensions into a shared directory: source/extensions/common/opentelemetry/.
Updates or adds documentation (README.md, design artifacts) in the new directory.

Risk Level: Low (organizational refactor, covered by existing tests)

Testing:

Ran existing unit and integration tests for tracing extensions.
Verified build and linting (buildifier, clang-format) passes.

Docs Changes:

Added/updated source/extensions/common/opentelemetry/README.md describing new organization and conventions.
Moved the mutualization plan to this directory.

Release Notes:

area: tracing
change: |
Moved all OpenTelemetry extension source and tests to source/extensions/common/opentelemetry/
and test/extensions/common/opentelemetry/ for better reuse and organization across different
extension types (tracers, access loggers, stat sinks). Updated OpenTelemetry Logs SDK to use
the new Common namespace structure for consistency.

Related to #41010

Please review and provide feedback!

…ommon/opentelemetry Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>

kyessenov

I have a general question about the direction. What do we gain by creating an internal SDK for OTEL? It seems that there's only few usages of it, and it would be hard to enforce that all usages use the SDK since they are mostly constants.

I agree we can organize some code (e.g. resource detectors should be common), and maybe attribute handling code is duplicated somewhere. But I don't understand the value of moving the constants when it's only used once.

kyessenov · 2025-09-18T16:31:33Z

changelogs/current.yaml

 new_features:
+- area: tracing
+  change: |
+    Moved all OpenTelemetry extension source and tests to source/extensions/common/opentelemetry/


Please put double backticks around symbols (source/...).

kyessenov · 2025-09-18T16:34:48Z

source/extensions/common/opentelemetry/ALIGNMENT_ANALYSIS.md

@@ -0,0 +1,386 @@
+# OpenTelemetry Alignment Analysis for Envoy


Is this AI generated? I don't think we should commit this file.

kyessenov · 2025-09-18T16:35:33Z

source/extensions/common/opentelemetry/BUILD

+envoy_extension_package()
+
+envoy_cc_library(
+    name = "opentelemetry_lib",


This is unnecessary, please use the direct deps instead.

kyessenov · 2025-09-18T16:36:33Z

source/extensions/stat_sinks/open_telemetry/open_telemetry_impl.cc

      service_method_(*Protobuf::DescriptorPool::generated_pool()->FindMethodByName(
-          "opentelemetry.proto.collector.metrics.v1.MetricsService.Export")) {}
+          std::string(Envoy::Extensions::Common::OpenTelemetry::Sdk::Metrics::Constants::
+                          METRICS_SERVICE_EXPORT_METHOD))) {}


You can drop Envoy::Extensions:: I think, since the final string is longer than the original :)

Also, are you sure you need std::string(? I think gRPC takes string_views these days.

kyessenov · 2025-09-18T16:37:57Z

CODEOWNERS

 /*/source/extensions/listener_managers/validation_listener_manager @adisuissa @ggreenway
 /*/source/extensions/config_subscription/ @adisuissa @UNOWNED
 /*/source/extensions/config_subscription/grpc @adisuissa @UNOWNED
+/*/extensions/common/opentelemetry @UNOWNED @UNOWNED


Why is it unowned? You can put my name and your name.

kyessenov · 2025-09-18T16:39:21Z

source/extensions/common/opentelemetry/exporters/otlp/README.md

@@ -0,0 +1,31 @@
+# OpenTelemetry OTLP Exporters


Thanks for adding these, but I think we don't need this level of details since it will get out of sync with the code. I think the top-level README is probably enough.

kyessenov · 2025-09-18T16:40:19Z

source/extensions/common/opentelemetry/sdk/trace/types.h

+using OTelSpanKind = ::opentelemetry::proto::trace::v1::Span::SpanKind;
+
+// OTLP export request/response types for traces (from OTLP protocol)
+using ExportRequest = ::opentelemetry::proto::collector::trace::v1::ExportTraceServiceRequest;


My general feeling is that we only gain benefits from a constant is when there is more than one usage. For these RPC stubs, there's really only one place they are used, which is the various sinks.

kyessenov · 2025-09-18T16:41:24Z

source/extensions/common/opentelemetry/sdk/metrics/types.h

+/**
+ * @brief Aggregation temporality for metrics (from OpenTelemetry specification)
+ */
+using AggregationTemporality = ::opentelemetry::proto::metrics::v1::AggregationTemporality;


IMO, these are unnecessary indirections. These protos are v1 so they will not change their location.

kyessenov · 2025-09-18T16:42:48Z

source/extensions/common/opentelemetry/sdk/common/types.h

+namespace Common {
+namespace OpenTelemetry {
+namespace Sdk {
+namespace Common {


I'd focus on these instead - these seem to be reused a lot. I'd recommend flattening the namespace, to just OpenTelemetry or Otel.

kyessenov · 2025-09-18T16:43:15Z

source/extensions/common/opentelemetry/sdk/common/types.h

+/**
+ * @brief Container holding Open-telemetry Attributes (Envoy extension)
+ */
+using OTelAttributes = std::map<std::string, OTelAttribute>;


why std map? flat_hash_map would be better?

kyessenov · 2025-09-18T16:54:05Z

/wait

github-actions · 2025-10-18T20:01:06Z

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

github-actions · 2025-10-26T00:04:19Z

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

mmorel-35 force-pushed the common/opentelemetry branch 9 times, most recently from df991ee to 0e16d6b Compare September 7, 2025 13:46

mmorel-35 marked this pull request as ready for review September 7, 2025 13:54

mmorel-35 requested review from mattklein123 and yanavlasov as code owners September 7, 2025 13:54

mmorel-35 marked this pull request as draft September 7, 2025 19:32

mmorel-35 force-pushed the common/opentelemetry branch 4 times, most recently from c5d5878 to 2b2dd1d Compare September 8, 2025 15:36

mmorel-35 changed the title ~~Centralize OpenTelemetry Constants and Helpers in source/common/opentelemetry~~ Centralize OpenTelemetry Constants and Helpers in source/extensions/opentelemetry Sep 8, 2025

mmorel-35 force-pushed the common/opentelemetry branch 12 times, most recently from f40288e to 266b436 Compare September 8, 2025 20:29

mmorel-35 force-pushed the common/opentelemetry branch 4 times, most recently from 9af799f to 7d428c2 Compare September 8, 2025 23:00

mmorel-35 changed the title ~~Centralize OpenTelemetry Constants and Helpers in source/extensions/opentelemetry~~ Centralize OpenTelemetry Constants and Helpers in source/extensions/common/opentelemetry Sep 9, 2025

mmorel-35 force-pushed the common/opentelemetry branch 7 times, most recently from 36929e8 to 258e22c Compare September 9, 2025 07:59

mmorel-35 marked this pull request as ready for review September 9, 2025 08:40

mmorel-35 force-pushed the common/opentelemetry branch 2 times, most recently from c7eff05 to cc6686e Compare September 9, 2025 15:36

mmorel-35 requested a review from kyessenov September 9, 2025 15:39

Centralize OpenTelemetry Constants and Helpers in source/extensions/c…

6146278

…ommon/opentelemetry Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>

mmorel-35 force-pushed the common/opentelemetry branch from cc6686e to 6146278 Compare September 9, 2025 15:45

mattklein123 assigned kyessenov Sep 18, 2025

kyessenov requested changes Sep 18, 2025

View reviewed changes

repokitteh-read-only bot added the waiting label Sep 18, 2025

github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Oct 18, 2025

github-actions bot closed this Oct 26, 2025

kamalmarhubi mentioned this pull request Apr 10, 2026

OpenTelemetry code mutualization: strategy for centralization and protocol support #41010

Open

mmorel-35 deleted the common/opentelemetry branch April 10, 2026 07:27

Copilot AI mentioned this pull request Apr 10, 2026

Centralize shared OpenTelemetry code into source/extensions/common/opentelemetry/ mmorel-35/envoy#187

Merged

mmorel-35 mentioned this pull request Apr 10, 2026

Centralize shared OpenTelemetry code into source/extensions/common/opentelemetry/ #44379

Open

		@@ -0,0 +1,386 @@
		# OpenTelemetry Alignment Analysis for Envoy

Conversation

mmorel-35 commented Sep 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kyessenov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyessenov commented Sep 18, 2025

Uh oh!

github-actions bot commented Oct 18, 2025

Uh oh!

github-actions bot commented Oct 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mmorel-35 commented Sep 7, 2025 •

edited

Loading