Add OTLP Ingestion endpoint #12571
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| ## Copying from opentelemetry/opentelemetry-collector-contrib | ||
|
|
||
| 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`. | ||
|
There was a problem hiding this comment. I am the author the normalization of metrics and labels of the Prometheus exporters in the OpenTelemetry Contrib project. Several comments:
There was a problem hiding this comment. Thanks for the original package and the review!
Already done :) We don't want our users to disable the feature gate, hence its removed, and the functionality is enabled by default.
Interesting, can you give an example?
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? There was a problem hiding this comment. 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 We could decide to simplify 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 ;-) |
||
| We don't copy in `./prometheus` through this script because that package imports a collector specific featuregate package we don't want to import. The featuregate package is being removed now, and in the future we will copy this folder too. | ||
|
|
||
| To update the dependency is a multi-step process: | ||
| 1. Vendor the latest `prometheus/prometheus`@`main` into [`opentelemetry/opentelemetry-collector-contrib`](https://github.com/open-telemetry/opentelemetry-collector-contrib) | ||
| 1. Update the VERSION in `update-copy.sh`. | ||
| 1. Run `./update-copy.sh`. | ||
|
|
||
| ### Why copy? | ||
|
|
||
| This is because the packages we copy depend on the [`prompb`](https://github.com/prometheus/prometheus/blob/main/prompb) package. While the package is relatively stable, there are still changes. For example, https://github.com/prometheus/prometheus/pull/11935 changed the types. | ||
| This means if we depend on the upstream packages directly, we will never able to make the changes like above. Hence we're copying the code for now. | ||
|
|
||
| ### I need to manually change these files | ||
|
|
||
| When we do want to make changes to the types in `prompb`, we might need to edit the files directly. That is OK, please let @gouthamve or @jesusvazquez know so they can take care of updating the upstream code (by vendoring in `prometheus/prometheus` upstream and resolving conflicts) and then will run the copy | ||
| script again to keep things updated. | ||
|
|
||
| [^1]: https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/pkg/translator/prometheus and https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/pkg/translator/prometheusremotewrite | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package normalize | ||
|
|
||
| import ( | ||
| "strings" | ||
| "unicode" | ||
| ) | ||
|
|
||
| // Normalizes the specified label to follow Prometheus label names standard | ||
| // | ||
| // See rules at https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels | ||
| // | ||
| // Labels that start with non-letter rune will be prefixed with "key_" | ||
| // | ||
| // Exception is made for double-underscores which are allowed | ||
| func NormalizeLabel(label string) string { | ||
| // Trivial case | ||
| if len(label) == 0 { | ||
| return label | ||
| } | ||
|
|
||
| // Replace all non-alphanumeric runes with underscores | ||
| label = strings.Map(sanitizeRune, label) | ||
|
|
||
| // If label starts with a number, prepend with "key_" | ||
| if unicode.IsDigit(rune(label[0])) { | ||
| label = "key_" + label | ||
| } | ||
|
|
||
| return label | ||
| } | ||
|
|
||
| // Return '_' for anything non-alphanumeric | ||
| func sanitizeRune(r rune) rune { | ||
| if unicode.IsLetter(r) || unicode.IsDigit(r) { | ||
| return r | ||
| } | ||
| return '_' | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package normalize | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestSanitizeDropSanitization(t *testing.T) { | ||
| require.Equal(t, "", NormalizeLabel("")) | ||
| require.Equal(t, "_test", NormalizeLabel("_test")) | ||
| require.Equal(t, "key_0test", NormalizeLabel("0test")) | ||
| require.Equal(t, "test", NormalizeLabel("test")) | ||
| require.Equal(t, "test__", NormalizeLabel("test_/")) | ||
| require.Equal(t, "__test", NormalizeLabel("__test")) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍