Add OTLP Ingestion endpoint #12571

gouthamve · 2023-07-17T19:22:42Z

We copy files from the otel-collector-contrib. See the README in storage/remote/otlptranslator/README.md.

This supersedes: #11965

jesusvazquez

Looking good, need to fix a few small lint issues and I think we're ready to merge this over.

roidelapluie

Can this wait 2 days so we first release 2.46? that gives us 6 weeks to polish

gouthamve · 2023-07-17T19:57:36Z

Can absolutely wait. No need to rush this :)

We copy files from the otel-collector-contrib. See the README in `storage/remote/otlptranslator/README.md`. This supersedes: prometheus#11965 Signed-off-by: gouthamve <[email protected]>

cstyan · 2023-07-17T22:31:52Z

.github/CODEOWNERS

@@ -1,6 +1,7 @@
 /web/ui @juliusv
 /web/ui/module @juliusv @nexucis
 /storage/remote @csmarchbanks @cstyan @bwplotka @tomwilkie
+/storage/remote/otlptranslator @gouthamve @jesusvazquez


cstyan · 2023-07-17T22:41:07Z

Has the conversation been started within in OTEL about moving the translator code so that we can remove the copy/paste code?

gouthamve · 2023-07-18T07:08:36Z

Yes, see: https://cloud-native.slack.com/archives/C01LSCJBXDZ/p1689161965395529

They'd like to see more involvement from the Prometheus maintainers before they're okay with moving the code.

gouthamve · 2023-07-25T21:16:49Z

I think this is ready to go. I've tested it by pushing metrics from a test application and the otel collector.

Found a bug in the OTEL Golang SDK in process too and updated the PR to account for it: open-telemetry/opentelemetry-go#4363

It is what the OTEL Golang SDK expect :( open-telemetry/opentelemetry-go#4363 Signed-off-by: Goutham <[email protected]>

roidelapluie · 2023-07-28T10:13:03Z

web/api/v1/api.go

@@ -380,6 +390,7 @@ func (api *API) Register(r *route.Router) {
 	r.Get("/status/walreplay", api.serveWALReplayStatus)
 	r.Post("/read", api.ready(api.remoteRead))
 	r.Post("/write", api.ready(api.remoteWrite))
+	r.Post("/otlp/v1/metrics", api.ready(api.otlpWrite))


Why did we go for this strange URL?
Is this standard?

I believe this is because typically the OTEL collector expects a single URL for all 3 signals (logs, traces, metrics) and then adds suffixes for each signal i.e v1/metrics.

roidelapluie

LGTM, I just added one question.

jesusvazquez

LGTM

roidelapluie · 2023-07-28T10:29:47Z

gouthamve · 2023-07-28T10:35:41Z

Now to write the documentation for this :)

prathamesh-sonpatki · 2023-07-29T11:52:13Z

@gouthamve and folks - Thanks for this ❤️ Please let me know if I can help in any way writing documentation for this :)

bertysentry · 2023-07-31T18:42:55Z

storage/remote/otlptranslator/README.md

+
+This files in the `prometheus/` and `prometheusremotewrite/` are copied from the OpenTelemetry Project[^1].
+
+This is done instead of adding a go.mod dependency because OpenTelemetry depends on `prometheus/prometheus` and a cyclic dependency will be created. This is just a temporary solution and the long-term solution is to move the required packages from OpenTelemetry into `prometheus/prometheus`. 


I am the author the normalization of metrics and labels of the Prometheus exporters in the OpenTelemetry Contrib project.

Several comments:

I hope you guys intend to enable the code that used to be behind the feature gate, to make sure Otel metrics are converted to proper Prometheus metrics that follow the metrics naming conventions (with unit, etc.).

OpenTelemetry plans to use namespaces everywhere in metric attributes, which will inevitably lead to very long labels in Prometheus. We may want to consider dropping the namespace prefixes in Otel attributes to get "simpler" labels.

To improve the performance, I wanted to create a sort of cache in the the translation package, in the form of a dictionary associating a triplet (metric name/type/unit) to the corresponding Prometheus metric name. I think that would be a nice addition.

Thanks for the original package and the review!

I hope you guys intend to enable the code that used to be behind the feature gate

Already done :) We don't want our users to disable the feature gate, hence its removed, and the functionality is enabled by default.

We may want to consider dropping the namespace prefixes in Otel attributes to get "simpler" labels.

Interesting, can you give an example?

I wanted to create a sort of cache in the the translation package, in the form of a dictionary associating a triplet (metric name/type/unit) to the corresponding Prometheus metric name.

That sounds great. We're using the same package in Mimir and I think we're seeing CPU cycles being spent on the string replaces. Looking it up using a cache sounds good. cc @bboreham @charleskorn

Do you still have plans to implement it upstream? If no, could you open an issue to track it, and we'll try to get someone to work on it?

Otel Technical Committee recently decided to enforce namespace prefixes for all metrics attributes. All metrics semantic conventions from OpenTelemetry are expected to be updated accordingly in the future.

For example, for JVM metrics: the process.runtime.jvm.gc.duration metric will come with the process.runtime.jvm.gc.name
and process.runtime.jvm.gc.action attributes. This will end up as process_runtime_jvm_gc_duration_seconds_counter{process_runtime_jvm_gc_name="...", process_runtime_jvm_gc_action="..."} in PromQL.

We could decide to simplify process_runtime_jvm_gc_name and process_runtime_jvm_gc_action labels to name and action respectively by removing the matching namespaces in metric name and its labels.

The benefit is that we get metrics and labels that look much like "classic" Prometheus metrics and labels. The problem is that the behavior could be counter-intuitive and difficult to predict in some cases (which labels get simplified or not?).

IMHO, everything would be better if OpenTelemetry didn't enforce namespaces in metrics attributes, but that boat has sailed 😅

Regarding the cache mechanism, I still had hopes to eventually implement it, but who am I kidding, I've been considering this for a year now, and haven't done a thing. I'll open an issue here ;-)

New issue: #12627

bwplotka · 2023-08-01T06:23:01Z

Thanks everybody for great work on this! 💪🏽

mx-psi · 2023-08-01T09:49:03Z

I filed #12623 so that Prometheus uses a more recent version of pdata on its implementation.

ArvinZheng · 2023-10-17T16:45:49Z

storage/remote/otlptranslator/prometheusremotewrite/number_data_points.go

+	prometheustranslator "github.com/prometheus/prometheus/storage/remote/otlptranslator/prometheus"
+)
+
+// addSingleSumNumberDataPoint converts the Gauge metric data point to a


incorrect method name in the comment

gouthamve requested a review from jesusvazquez July 17, 2023 19:22

gouthamve requested review from csmarchbanks, cstyan, bwplotka and tomwilkie as code owners July 17, 2023 19:22

jesusvazquez reviewed Jul 17, 2023

View reviewed changes

roidelapluie reviewed Jul 17, 2023

View reviewed changes

gouthamve force-pushed the otlp-gateway-take-2 branch 3 times, most recently from 0a36aa0 to 2f04ded Compare July 17, 2023 21:54

Add OTLP Ingestion endpoint

c055cd1

We copy files from the otel-collector-contrib. See the README in `storage/remote/otlptranslator/README.md`. This supersedes: prometheus#11965 Signed-off-by: gouthamve <[email protected]>

gouthamve force-pushed the otlp-gateway-take-2 branch from 2f04ded to c055cd1 Compare July 17, 2023 21:59

cstyan reviewed Jul 17, 2023

View reviewed changes

Return a 200 OK

08a2145

It is what the OTEL Golang SDK expect :( open-telemetry/opentelemetry-go#4363 Signed-off-by: Goutham <[email protected]>

gouthamve force-pushed the otlp-gateway-take-2 branch from 6b6a924 to 08a2145 Compare July 25, 2023 21:17

roidelapluie reviewed Jul 28, 2023

View reviewed changes

roidelapluie approved these changes Jul 28, 2023

View reviewed changes

jesusvazquez approved these changes Jul 28, 2023

View reviewed changes

gouthamve merged commit ad4f514 into prometheus:main Jul 28, 2023

gouthamve deleted the otlp-gateway-take-2 branch July 28, 2023 10:35

yeya24 mentioned this pull request Jul 28, 2023

OTLP native support cortexproject/cortex#4981

Closed

bertysentry reviewed Jul 31, 2023

View reviewed changes

mx-psi mentioned this pull request Aug 1, 2023

[go.mod] Bump go.opentelemetry.io/collector/pdata to v1.0.0-rcv0014 #12623

Merged

bertysentry mentioned this pull request Aug 1, 2023

Performance: when ingesting Otel metrics, use a cache for metric and labels normalization #12627

Closed

gouthamve mentioned this pull request Aug 1, 2023

prometheus: Remove namespace on attributes open-telemetry/opentelemetry-specification#3634

Open

arukiidou mentioned this pull request Aug 2, 2023

Document OTLP ingestion endpoint #12642

Open

3 tasks

simonpasquier modified the milestone: OTEL Support Aug 7, 2023

ArvinZheng reviewed Oct 17, 2023

View reviewed changes

KasonBraley mentioned this pull request Jan 19, 2024

feat: Expose pgxpool metrics exaring/otelpgx#18

Open

davorrunje mentioned this pull request Jan 23, 2024

Logging microsoft/autogen#1146

Merged

14 tasks

cylewitruk mentioned this pull request Sep 3, 2024

[Feature]: Add metrics and other monitoring for the signer stacks-network/sbtc#481

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OTLP Ingestion endpoint #12571

Add OTLP Ingestion endpoint #12571

gouthamve commented Jul 17, 2023

jesusvazquez left a comment

roidelapluie left a comment

gouthamve commented Jul 17, 2023

cstyan Jul 17, 2023

cstyan commented Jul 17, 2023

gouthamve commented Jul 18, 2023 •

edited

Loading

gouthamve commented Jul 25, 2023

roidelapluie Jul 28, 2023

jesusvazquez Jul 28, 2023

roidelapluie left a comment

jesusvazquez left a comment

roidelapluie commented Jul 28, 2023

gouthamve commented Jul 28, 2023

prathamesh-sonpatki commented Jul 29, 2023

bertysentry Jul 31, 2023

gouthamve Jul 31, 2023

bertysentry Aug 1, 2023 •

edited

Loading

bertysentry Aug 1, 2023

bwplotka commented Aug 1, 2023

mx-psi commented Aug 1, 2023

ArvinZheng Oct 17, 2023


		This files in the `prometheus/` and `prometheusremotewrite/` are copied from the OpenTelemetry Project[^1].

		This is done instead of adding a go.mod dependency because OpenTelemetry depends on `prometheus/prometheus` and a cyclic dependency will be created. This is just a temporary solution and the long-term solution is to move the required packages from OpenTelemetry into `prometheus/prometheus`.

Add OTLP Ingestion endpoint #12571

Add OTLP Ingestion endpoint #12571

Conversation

gouthamve commented Jul 17, 2023

jesusvazquez left a comment

Choose a reason for hiding this comment

roidelapluie left a comment

Choose a reason for hiding this comment

gouthamve commented Jul 17, 2023

cstyan Jul 17, 2023

Choose a reason for hiding this comment

cstyan commented Jul 17, 2023

gouthamve commented Jul 18, 2023 • edited Loading

gouthamve commented Jul 25, 2023

roidelapluie Jul 28, 2023

Choose a reason for hiding this comment

jesusvazquez Jul 28, 2023

Choose a reason for hiding this comment

roidelapluie left a comment

Choose a reason for hiding this comment

jesusvazquez left a comment

Choose a reason for hiding this comment

roidelapluie commented Jul 28, 2023

gouthamve commented Jul 28, 2023

prathamesh-sonpatki commented Jul 29, 2023

bertysentry Jul 31, 2023

Choose a reason for hiding this comment

gouthamve Jul 31, 2023

Choose a reason for hiding this comment

bertysentry Aug 1, 2023 • edited Loading

Choose a reason for hiding this comment

bertysentry Aug 1, 2023

Choose a reason for hiding this comment

bwplotka commented Aug 1, 2023

mx-psi commented Aug 1, 2023

ArvinZheng Oct 17, 2023

Choose a reason for hiding this comment

gouthamve commented Jul 18, 2023 •

edited

Loading

bertysentry Aug 1, 2023 •

edited

Loading