Skip to content

Conversation

SungJin1212
Copy link
Member

Add an otlp exemplar ingestion test.

Which issue(s) this PR fixes:
Fixes #6235

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@SungJin1212
Copy link
Member Author

SungJin1212 commented Sep 27, 2024

@yeya24
I think an otlp metadata ingestion test is needed as well. Let me add it.

@yeya24
Copy link
Contributor

yeya24 commented Sep 27, 2024

@SungJin1212 I don't think OTLP protocol supports metadata as metadata is only a Prometheus thing

@SungJin1212
Copy link
Member Author

@yeya24
I just found out the otlp ingestion of the metadata is not implemented yet. Does that mean that the implementation itself is not necessary?

@yeya24
Copy link
Contributor

yeya24 commented Sep 27, 2024

https://github.com/prometheus/prometheus/blob/main/storage/remote/write_handler.go#L527
If you look at the Prometheus OTLP handler implementation it only ingests timeseries.

https://opentelemetry.io/docs/specs/otlp/
Metadata is only part of Prometheus remote write protocol but not part of OTLP specification so we can skip

@SungJin1212
Copy link
Member Author

@yeya24
I got it. Thank you!

@yeya24
Copy link
Contributor

yeya24 commented Sep 27, 2024

prometheus/prometheus#13439 @SungJin1212 We can cover metadata API after this upstream PR gets merged

Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Great work. Just some nits

@yeya24 yeya24 merged commit ff782ca into cortexproject:master Sep 27, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Add exemplars coverage in OTLP integration test

2 participants