Skip to content

Fix streams limit error check so that metrics are correctly labeled as ReasonStreamLimited#3466

Merged
kalleep merged 3 commits intografana:mainfrom
maratkhv:fix/stream-limit-error-check
Apr 30, 2025
Merged

Fix streams limit error check so that metrics are correctly labeled as ReasonStreamLimited#3466
kalleep merged 3 commits intografana:mainfrom
maratkhv:fix/stream-limit-error-check

Conversation

@maratkhv
Copy link
Contributor

@maratkhv maratkhv commented Apr 29, 2025

PR Description

This PR fixes an issue where such error check
if err.Error() == errMaxStreamsLimitExceeded { reason = ReasonStreamLimited }
would always fail because batch.add() returns a formatted error, so the metrics are not labeled properly.

Which issue(s) this PR fixes

Notes to the Reviewer

  • I believe using a regexp here would be overkill; a simple HasPrefix check is sufficient.

PR Checklist

  • CHANGELOG.md updated
  • Tests updated

@maratkhv maratkhv requested a review from a team as a code owner April 29, 2025 01:27
@maratkhv maratkhv requested a review from kalleep April 29, 2025 09:37
Copy link
Contributor

@kalleep kalleep left a comment

Choose a reason for hiding this comment

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

LGTM, we are sorting out some issues with our ci. but once those are resolved you will need to rebase and we can merge after that. Will ping you when everything is ready

@kalleep
Copy link
Contributor

kalleep commented Apr 29, 2025

@maratkhv mind doing a rebase on main?

@maratkhv maratkhv force-pushed the fix/stream-limit-error-check branch from 87cdc52 to e0e78e6 Compare April 29, 2025 15:45
@maratkhv maratkhv requested a review from kalleep April 29, 2025 15:52
@kalleep kalleep merged commit 17835a4 into grafana:main Apr 30, 2025
32 checks passed
@maratkhv maratkhv deleted the fix/stream-limit-error-check branch April 30, 2025 14:37
kalleep pushed a commit that referenced this pull request May 5, 2025
…s `ReasonStreamLimited` (#3466)

* fix: replace direct error string compare with isErrMaxStreamsLimitExceeded helper

* update CHANGELOG

* Make errMaxStreamsLimitExceeded an error type
kalleep added a commit that referenced this pull request May 5, 2025
* fix: panic that happens when a target gets deleted when using decompression (#3475)

* Fix panic caused that can happen when when file is removed for
decompressor

* Change to start readLine start and stops updatePositions

* Add changelog

* Fix mimir.rules.kubernetes panic on non-leader debug info retrieval (#3451)

* Fix mimir.rules.kubernetes to only return eventProcessor state if it exists

* fix: deadlock in loki.source.file when target is removed (#3488)

* Fix deadlock that can happen when stopping reader tasks

Co-authored-by: William Dumont <william.dumont@grafana.com>

* fix: emit valid logfmt key (#3495)

* Fix log keys to be valid for logfmt

* Add changelog

* Fix streams limit error check so that metrics are correctly labeled as `ReasonStreamLimited` (#3466)

* fix: replace direct error string compare with isErrMaxStreamsLimitExceeded helper

* update CHANGELOG

* Make errMaxStreamsLimitExceeded an error type

---------

Co-authored-by: Théo Brigitte <theo.brigitte@gmail.com>
Co-authored-by: William Dumont <william.dumont@grafana.com>
Co-authored-by: Marat Khvostov <marathvostov@gmail.com>
marctc pushed a commit that referenced this pull request May 7, 2025
…s `ReasonStreamLimited` (#3466)

* fix: replace direct error string compare with isErrMaxStreamsLimitExceeded helper

* update CHANGELOG

* Make errMaxStreamsLimitExceeded an error type
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 31, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants