fix(telemetry): export properly resources configured on prometheus#7394
fix(telemetry): export properly resources configured on prometheus#7394
Conversation
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 1 changed, 0 removedBuild ID: 2a5812e2b6ed7af8c5659ab8 URL: https://www.apollographql.com/docs/deploy-preview/2a5812e2b6ed7af8c5659ab8 |
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
...elemetry/snapshots/apollo_router__plugins__telemetry__tests__it_test_prometheus_metrics.snap
Outdated
Show resolved
Hide resolved
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
There was a problem hiding this comment.
I'm not clear this is the best way to go, usually target_info has the resource info:
# HELP target_info Target metadata
# TYPE target_info gauge
target_info{process_executable_name="router",service_name="unknown_service:router",service_version="2.2.1",test="test"} 1
…telemetry_prom_resource
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
|
I think all that's needed is that this is a config option in the prometheus config. Here's the relevant part of the spec:
So to merge the PR is to move away from the "SHOULD" behaviour. We can add an option on the prometheus exporter for this to fulfil: |
|
Suggest: |
|
@BrynCooke Still unsure to understand because if I set this configuration to |
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
…graphql/router into bnjjj/fix_telemetry_prom_resource
Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>
BrynCooke
left a comment
There was a problem hiding this comment.
You just need to add docs for the new option
…7394) Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com> (cherry picked from commit 731fd23) # Conflicts: # apollo-router/src/plugins/telemetry/metrics/prometheus.rs # apollo-router/src/plugins/telemetry/mod.rs # apollo-router/src/plugins/telemetry/snapshots/apollo_router__plugins__telemetry__tests__it_test_prometheus_metrics.snap # apollo-router/src/plugins/telemetry/snapshots/apollo_router__plugins__telemetry__tests__it_test_prometheus_metrics_units_are_included.snap # apollo-router/tests/integration/telemetry/metrics.rs # docs/source/routing/observability/telemetry/metrics-exporters/prometheus.mdx
When configuring
resourceto globally add labels on metrics like this:test-resourcelabel was never exported to prometheus, this bug only occurs with prometheus and not otlp.This PR fixes this behavior and will no longer filter
resources.Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
Exceptions
Note any exceptions here
Notes
Footnotes
It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. ↩
Configuration is an important part of many changes. Where applicable please try to document configuration examples. ↩
Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩