Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[receiver/prometheusreceiver] Exemplar Support #14132

Merged
merged 4 commits into from
Oct 13, 2022
Merged

[receiver/prometheusreceiver] Exemplar Support #14132

merged 4 commits into from
Oct 13, 2022

Conversation

ghost
Copy link

@ghost ghost commented Sep 15, 2022

Description:
Add exemplar support to Prometheus Receiver

Link to tracking Issue:
#8353

Testing:
Added multiple unit tests for all possible scenarios

  1. If exemplars are sent as null
  2. If AppendExemplar method is called without calling Append method
  3. If exemplars are sent without labels, metric name, empty metric name
  4. If exemplars are sent with duplicate labels
  5. If exemplars are sent without value, time stamp.
  6. If exemplars are sent without trace id, span id
  7. If exemplars are sent with length of trace id > 16 and span id > 8 bytes
  8. If exemplars are sent with length of trace id < 16 and span id < 8

@ghost ghost self-requested a review September 15, 2022 02:36
@ghost ghost requested review from Aneurysm9 and dashpole as code owners September 15, 2022 02:36
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 15, 2022 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: ayandapalli (944e94ac27363e6183fd11557748fd01df15aed5, a93df877e372f04dca09056e817acfdb6e851812, 8f1d61d017a487761639571af1144efe36df5dbf, eea3d8ffa04667d0c2bb3859e6a63cf61f4edf32, 8bf1229b51cc3641de5ab075fa0042b8543019c7, 58805d0e3eb0c0665f17cba06de90beb96ea6fa9, b6b20450e2f892dd7a1139745f675bfef5de98dc)

@ghost ghost changed the title Prom receiver exemplars [receiver/prometheusreceiver] Exemplar Support Sep 15, 2022
bogdandrutu
bogdandrutu previously requested changes Sep 15, 2022
View reviewed changes
receiver/prometheusreceiver/internal/metricfamily.go Outdated Show resolved Hide resolved
receiver/prometheusreceiver/internal/metricfamily.go Outdated Show resolved Hide resolved
receiver/prometheusreceiver/internal/metricfamily.go Outdated Show resolved Hide resolved
receiver/prometheusreceiver/internal/metricfamily.go Outdated Show resolved Hide resolved
receiver/prometheusreceiver/internal/metricfamily.go Outdated Show resolved Hide resolved
receiver/prometheusreceiver/internal/transaction_test.go Outdated Show resolved Hide resolved
receiver/prometheusreceiver/internal/transaction_test.go Outdated Show resolved Hide resolved
receiver/prometheusreceiver/internal/transaction_test.go Outdated Show resolved Hide resolved
receiver/prometheusreceiver/internal/transaction_test.go Outdated Show resolved Hide resolved
receiver/prometheusreceiver/internal/transaction_test.go Outdated Show resolved Hide resolved
@ghost ghost force-pushed the prom-receiver-exemplars branch from b6b2045 to 09d72f5 Compare September 15, 2022 18:13
@ghost
Copy link
Author

ghost commented Sep 15, 2022 •
edited by ghost
Loading

@bogdandrutu Thanks a lot for the review. Addressed the comments

@bogdandrutu
Copy link
Member

@ayandapalli you should comment on all my comments, and resolve them when you did.

bogdandrutu
bogdandrutu reviewed Sep 15, 2022
View reviewed changes
receiver/prometheusreceiver/internal/metricfamily.go
Comment on lines +151 to +158
if mg == nil {
return
}
Copy link
Member

Choose a reason for hiding this comment

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

How can this happen? We don't do this check in any other func.

Copy link
Author

Choose a reason for hiding this comment

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

Since we are accessing mg.exemplars in the nextline, have this check just to avoid any kind of nil access even in the future

receiver/prometheusreceiver/internal/metricfamily.go Outdated Show resolved Hide resolved
receiver/prometheusreceiver/internal/metricfamily.go Outdated Show resolved Hide resolved
receiver/prometheusreceiver/internal/metricfamily.go Outdated Show resolved Hide resolved
receiver/prometheusreceiver/internal/transaction.go Outdated Show resolved Hide resolved
receiver/prometheusreceiver/internal/transaction_test.go Outdated Show resolved Hide resolved
@ghost ghost requested review from djaglowski, jmacd and dmitryax as code owners September 22, 2022 21:54
@ghost ghost self-requested a review September 22, 2022 21:54
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 22, 2022 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: ayandapalli (062b6a6b5e390f591c5649beae5bab01795341c0, 8f6ab4f709e80d848a1602ad5ee0e5c793e08bbf, 94b398b26672cfc5d803accf7978735d8af6f92b, b85f51b9d2ac2c2f7df21e629060c42d45bcce73, 3889fe3df32b0ff212beadc130a45780b08db2e8, 3a913bfbf6a35a4563f751dc64bae2f60ca013da, f88527d23a793cb83ed77263684886a6cab3c42c, 4ce1a76d733acd2abf64de41394af6c1981adf6a, 3f4227930aaa5ab63fec776792034e9bd24cef94, 0c082e930b14e55ce419f42c1afdb75e9e3c30bd, 9a1e9d2a4d106fb344768212749241604bf37087, 4c64965ee81972b79ac05b2a8a5502463b0ba470, 56196ff62b07b8d822520b25d141ba348c132527, fa05e27e1143f0ce2c8ec45f66799d9049a32963, 6c21ddb83341884fd0785dea40c77020dfd17465, ef30660d00b91b8d2efaf531fa0cf5da257799aa)

@ghost ghost force-pushed the prom-receiver-exemplars branch from e6d518a to ef30660 Compare September 22, 2022 22:02
@ghost ghost force-pushed the prom-receiver-exemplars branch 2 times, most recently from 35def41 to 139f831 Compare September 27, 2022 00:29
@ghost ghost force-pushed the prom-receiver-exemplars branch from dae482f to cde77f6 Compare October 6, 2022 17:28
@ghost
Copy link
Author

ghost commented Oct 7, 2022

@dashpole @Aneurysm9 Happy to address any comments further. Can you please help me to get this merged? Thanks in advance.

Aneurysm9
Aneurysm9 approved these changes Oct 7, 2022
View reviewed changes
receiver/prometheusreceiver/README.md Outdated Show resolved Hide resolved
@dashpole dashpole added receiver/prometheus Prometheus receiver ready to merge Code review completed; ready to merge by maintainers enhancement New feature or request labels Oct 12, 2022
@codeboten codeboten merged commit ca0f8f3 into open-telemetry:main Oct 13, 2022
@dashpole dashpole mentioned this pull request Oct 17, 2022
Closed
@jeromeinsf jeromeinsf mentioned this pull request Nov 10, 2022
Closed
shalper2 pushed a commit to shalper2/opentelemetry-collector-contrib that referenced this pull request Dec 6, 2022
@Aneurysm9 @shalper2
[receiver/prometheusreceiver] Exemplar Support (open-telemetry#14132)
d79e2c7 
 Add exemplar support to Prometheus Receiver

Co-authored-by: Anthony Mirabella <[email protected]>
@plantfansam plantfansam mentioned this pull request Jul 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djaglowski djaglowski Awaiting requested review from djaglowski

@jmacd jmacd Awaiting requested review from jmacd

@dmitryax dmitryax Awaiting requested review from dmitryax

@jpkrohling jpkrohling Awaiting requested review from jpkrohling

@mx-psi mx-psi Awaiting requested review from mx-psi

@TylerHelmuth TylerHelmuth Awaiting requested review from TylerHelmuth

@jsuereth jsuereth Awaiting requested review from jsuereth

@punya punya Awaiting requested review from punya

@arminru arminru Awaiting requested review from arminru

Assignees

@bogdandrutu bogdandrutu

Labels
enhancement New feature or request ready to merge Code review completed; ready to merge by maintainers receiver/prometheus Prometheus receiver
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bogdandrutu @Aneurysm9 @dashpole @codeboten @aishyandapalli