Skip to content
New issue

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

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

Already on GitHub? # to your account

(jaeger): Fix jaeger span metrics #1174

Merged
merged 4 commits into from
Oct 20, 2023
Merged

(jaeger): Fix jaeger span metrics #1174

merged 4 commits into from
Oct 20, 2023

Conversation

puckpuck
Copy link
Contributor

@puckpuck puckpuck commented Oct 13, 2023

Changes

Fixes: #864

Adds proper command args to Jaeger to understand the proper metric names generated from the spanmetrics connectors.

Merge Requirements

  • CHANGELOG.md updated to document new feature additions
  • Appropriate Helm chart updates in the helm-charts

Maintainers will not merge until the above have been completed. If you're unsure
which docs need to be changed ping the
@open-telemetry/demo-approvers.

Signed-off-by: Pierre Tessier <pierre@pierretessier.com>
@puckpuck puckpuck requested a review from a team October 13, 2023 04:22
Signed-off-by: Pierre Tessier <pierre@pierretessier.com>
Copy link
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

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

LGTM

@julianocosta89
Copy link
Member

@puckpuck just one question, what is the file: src/prometheus/prometheus-config.yaml that is empty but shouldn't be removed?

@puckpuck
Copy link
Contributor Author

@puckpuck just one question, what is the file: src/prometheus/prometheus-config.yaml that is empty but shouldn't be removed?

Prometheus will error out if a config file is not passed into the startup, since the default image does not have one. Typically Prometheus is used to pull metrics, so a config will always be present, but since we are using it only in a push method, we don't have a scraping config to pass in.

prometheus  | ts=2023-10-20T03:47:16.236Z caller=main.go:487 level=error msg="Error loading config (--config.file=prometheus.yml)" file=/prometheus/prometheus.yml err="open prometheus.yml: no such file or directory"

@puckpuck puckpuck merged commit c36cd98 into open-telemetry:main Oct 20, 2023
@puckpuck puckpuck deleted the o11y.jaeger-spm-prometheus branch October 20, 2023 13:40
Dylan-M pushed a commit to observIQ/opentelemetry-demo that referenced this pull request Oct 25, 2023
* jaeger spm and prometheus

Signed-off-by: Pierre Tessier <pierre@pierretessier.com>

* jaeger spm and prometheus

Signed-off-by: Pierre Tessier <pierre@pierretessier.com>

---------

Signed-off-by: Pierre Tessier <pierre@pierretessier.com>
Dylan-M pushed a commit to observIQ/opentelemetry-demo that referenced this pull request Oct 25, 2023
* jaeger spm and prometheus

Signed-off-by: Pierre Tessier <pierre@pierretessier.com>

* jaeger spm and prometheus

Signed-off-by: Pierre Tessier <pierre@pierretessier.com>

---------

Signed-off-by: Pierre Tessier <pierre@pierretessier.com>
@austinlparker
Copy link
Member

Turns out this file was kinda important as it was scraping collector metrics. Removing it broke the collector dashboard in Grafana.

jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
* jaeger spm and prometheus

Signed-off-by: Pierre Tessier <pierre@pierretessier.com>

* jaeger spm and prometheus

Signed-off-by: Pierre Tessier <pierre@pierretessier.com>

---------

Signed-off-by: Pierre Tessier <pierre@pierretessier.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jaeger Monitor not compatible with current spanmetrics setup
3 participants