Skip to content

Fix edge case in metrics collector#3875

Merged
tillprochaska merged 2 commits intodevelopfrom
fix/prometheus-metrics-edge-case
Oct 15, 2024
Merged

Fix edge case in metrics collector#3875
tillprochaska merged 2 commits intodevelopfrom
fix/prometheus-metrics-edge-case

Conversation

@tillprochaska
Copy link
Contributor

For some reason, the default/fallback value returned by get_collection_stats is an empty list, not an empty object. This can trigger an edge case if Prometheus metrics are scraped after a collection has been created, but the stats for the new collection haven’t been computed yet.

To prevent unintended side effects, I haven’t changed the default/fallback value (as other parts of the code base might rely on it) and have instead explicitly handled empty values.

For some reason, the default/fallback value returned by `get_collection_stats` is an empty list, not an empty object. This can trigger an edge case if Prometheus metrics are scraped after a collection has been created, but the stats for the new collection haven’t been computed yet.

To prevent unintended side effects, I haven’t changed the default/fallback value (as other parts of the code base might rely on it) and have instead explicitly handled empty values.
@tillprochaska tillprochaska marked this pull request as draft October 15, 2024 09:47
This shouldn’t make a difference, but unfortunately our test suite contains a bunch of tests that have side effects (e.g. they create entities that aren’t properly rolled back after the tests). In this particular case, other tests create Person entities, but not Airplane entities, so that’s a workaround for now.
@tillprochaska tillprochaska marked this pull request as ready for review October 15, 2024 14:49
Copy link
Contributor

@stchris stchris left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for the fix!

@stchris stchris self-requested a review October 15, 2024 14:52
@tillprochaska tillprochaska merged commit e81db80 into develop Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants