Skip to content

fix(config) - added missing opentelemetry options #1915

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

Closed
wants to merge 4 commits into from
Closed

fix(config) - added missing opentelemetry options #1915

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ee9efe8
fix(config) - added missing opentelemetry options - remapped port fro…
kkotowiczz Apr 22, 2025
8062fab
fix(config) - Included copyright
kkotowiczz Apr 22, 2025
af70744
fix(config) - Missing whitespace
kkotowiczz Apr 22, 2025
6397630
fix(config) - More detailed comment on port prometheus' remapping.
kkotowiczz Apr 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions charts/tractusx-connector/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# Copyright (c) 2023 Mercedes-Benz Tech Innovation GmbH
# Copyright (c) 2023 Bayerische Motoren Werke Aktiengesellschaft (BMW AG)
# Copyright (c) 2021,2024 Contributors to the Eclipse Foundation
# Copyright (c) 2025 Schaeffler AG
#
# See the NOTICE file(s) distributed with this work for additional
# information regarding copyright ownership.
Expand Down Expand Up @@ -333,6 +334,9 @@ controlplane:
opentelemetry: |-
otel.javaagent.enabled=false
otel.javaagent.debug=false
# By default prometheus uses port 9464
Copy link
Contributor

@lgblaumeiser lgblaumeiser Apr 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

One slight proposal on this comment and the same below, @kkotowiczz. I do not understand the point. Is it, that this config is only needed, because otel and prometheus are not aligned concerning port? Please adapt the comment in a way, that describes the issue better. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's just a configuration issue why is this change needed to happen here? tbh I don't see why we should have a particular exporter configured by default when that should be done by the user (in fact, the java agent is disabled by default, see line 335)

Copy link
Author

Choose a reason for hiding this comment

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

Ok, so it seems then like #1905 it's not a bug, because these are the only changes needed to scrap metrics with prometheus.

So I don't know how you proceed in similar cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can just provide documentation to tell users how they can have the /metrics endpoint exposed

Copy link
Author

Choose a reason for hiding this comment

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

Closing PR then

otel.exporter.prometheus.port=9090
otel.metrics.exporter=prometheus


# -- [node selector](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#nodeselector) to constrain pods to nodes
Expand Down Expand Up @@ -575,6 +579,9 @@ dataplane:
opentelemetry: |-
otel.javaagent.enabled=false
otel.javaagent.debug=false
# By default prometheus uses port 9464
otel.exporter.prometheus.port=9090
otel.metrics.exporter=prometheus


# -- [node selector](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#nodeselector) to constrain pods to nodes
Expand Down
Loading