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

Add default unknown_producer tag value to the producer tag of the StreamingTripUpdateMetrics #6513

Merged
merged 1 commit into from
Mar 6, 2025

Conversation

eibakke
Copy link
Contributor

@eibakke eibakke commented Mar 5, 2025

Summary

This PR sets a default value to the producer tag on the prometheus realtime metrics. The current state leaves the producer tag empty when there is no producer on the result, which leads to metrics not updating because prometheus metrics have to have the same tags.

Warning we get currently:

The meter (MeterId{name='streaming_trip_updates.failed', tags=[tag(configRef=SIRI_ET_GOOGLE_PUBSUB_UPDATER),tag(errorType=UNKNOWN),tag(feedId=RB),tag(url=http://realtime-cache.dev.entur.internal/et)]}) registration has failed: Prometheus requires that all meters with the same name have the same set of tag keys. There is already an existing meter named 'streaming_trip_updates.failed' containing tag keys [configRef, errorType, feedId, producer, url]. The meter you are attempting to register has keys [configRef, errorType, feedId, url]. Note that subsequent logs will be logged at debug level.\n

Unit tests

No tests for this part of the code

…reamingTripUpdateMetrics.

This because updates fail when prometheus metrics have tags on some updates but not on others.
@eibakke eibakke added the Bug label Mar 5, 2025
@eibakke eibakke requested a review from a team as a code owner March 5, 2025 14:36
Copy link

codecov bot commented Mar 5, 2025

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 70.18%. Comparing base (a555e95) to head (71d1223).
Report is 30 commits behind head on dev-2.x.

Files with missing lines Patch % Lines
...dater/trip/metrics/StreamingTripUpdateMetrics.java 0.00% 7 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #6513      +/-   ##
=============================================
- Coverage      70.18%   70.18%   -0.01%     
+ Complexity     18308    18307       -1     
=============================================
  Files           2081     2081              
  Lines          77184    77187       +3     
  Branches        7828     7828              
=============================================
  Hits           54172    54172              
- Misses         20246    20250       +4     
+ Partials        2766     2765       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vpaturet vpaturet added the Entur Test This is currently being tested at Entur label Mar 5, 2025
Copy link
Member

@leonardehrenfried leonardehrenfried left a comment

Choose a reason for hiding this comment

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

This looks fine. You say that metrics don't refresh. Does this mean that all metrics hang or just the trip update ones?

@vpaturet vpaturet self-requested a review March 6, 2025 13:32
@eibakke eibakke merged commit 63045ad into opentripplanner:dev-2.x Mar 6, 2025
6 checks passed
@eibakke eibakke deleted the tag_producer_empty_fix branch March 6, 2025 14:45
t2gran pushed a commit that referenced this pull request Mar 6, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Bug Entur Test This is currently being tested at Entur
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants