OTLP: Convert start timestamps to Prom created timestamps #9131
Conversation
aknuds1
force-pushed
the
arve/otlp-starttime
branch
4 times, most recently
from
afc98c1
to
cb93c3e
Compare
aknuds1
changed the title
aknuds1
force-pushed
the
arve/otlp-starttime
branch
from
cb93c3e
to
2925e8e
Compare
jesusvazquez
force-pushed
the
arve/otlp-starttime
branch
from
3876b9f
to
3937656
Compare
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>
jesusvazquez
force-pushed
the
arve/otlp-starttime
branch
from
84dd479
to
82e2b6a
Compare
|
This PR includes the latest changes from Mimir Prometheus so I'm waiting on #9148 before moving forward. |
| - Enable conversion of OTel start timestamps to Prometheus zero samples to mark series start | ||
| - `-distributor.otel-created-timestamp-zero-ingestion-enabled` |
There was a problem hiding this comment.
In OTel it is called StartTimeUnixNano, should we call this flag -distributor.otel-starttimeunixnano-zero-ingestion-enabled?
There was a problem hiding this comment.
As a point of reference, the OTel SDK calls it StartTimestamp. Do you gain anything from including the unixnano suffix? Technically, Prometheus created timestamps are in milliseconds, but there's no point in including that kind of detail when referring to them, is there?
I guess it might make sense to refer to "start timestamps" in this flag rather than "created timestamps", since you're converting the former. My original thinking, OTOH, was to refer to support for created timestamps via zero samples (since zero samples are a way to represent created timestamps) in the OTLP endpoint.
There was a problem hiding this comment.
As a point of reference, the OTel SDK calls it StartTimestamp. Do you gain anything from including the unixnano suffix?
No, you have a good point to not add unixnano!
I guess it might make sense to refer to "start timestamps" in this flag rather than "created timestamps", since you're converting the former. My original thinking, OTOH, was to refer to support for created timestamps via zero samples (since zero samples are a way to represent created timestamps) in the OTLP endpoint.
I don't have a strong preference, I was just thinking that people coming from OTel might not understand this flag. Quite unfortunate that OpenMetrics and OTel are using different names here :/
There was a problem hiding this comment.
Yeah, I guess you've got a point. Maybe e.g. -otel-start-time-zero-ingestion-enabled (same as yours, without "unixnano")?
# Conflicts: # go.mod # go.sum # pkg/distributor/otel.go # vendor/github.com/prometheus/prometheus/storage/remote/write_handler.go # vendor/modules.txt
| // client_golang v1.20.0 has some bugs https://github.com/prometheus/client_golang/issues/1605, https://github.com/prometheus/client_golang/issues/1607 | ||
| // Stick to v1.19.1 until they are fixed. | ||
| replace github.com/prometheus/client_golang => github.com/prometheus/client_golang v1.19.1 |
There was a problem hiding this comment.
Taking a look at them right now!
aknuds1
requested review from
stevesg,
grafanabot,
a team and
tacole02
as code owners
* OTLP: Convert start timestamps to Mimir created timestamps Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com> * bump to latest mimir-prometheus Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com> * make use of annotations --------- Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com> Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com> Co-authored-by: Jesus Vazquez <jesus.vazquez@grafana.com> Co-authored-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> (cherry picked from commit b58a08b)
) * OTLP: Convert start timestamps to Mimir created timestamps Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com> * bump to latest mimir-prometheus Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com> * make use of annotations --------- Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com> Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com> Co-authored-by: Jesus Vazquez <jesus.vazquez@grafana.com> Co-authored-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com> (cherry picked from commit b58a08b) Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-9131-to-r306 origin/r306
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x b58a08b17fbe7a53d59ca48995e57e38b893417e
# Push it to GitHub
git push --set-upstream origin backport-9131-to-r306
git switch main
# Remove the local backport branch
git branch -D backport-9131-to-r306Then, create a pull request where the |
What this PR does
In the OTLP endpoint, support converting OTel data point start timestamps to Mimir zero samples 1 millisecond before the sample itself, iff the following are true:
Includes grafana/mimir-prometheus#684.
Which issue(s) this PR fixes or relates to
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.