Skip to content
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.

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

Stream and polling processor logging updates to warn on recoverable errors #219

Merged
merged 2 commits into from
Sep 6, 2023
Merged

Stream and polling processor logging updates to warn on recoverable errors #219

merged 2 commits into from
Sep 6, 2023

Conversation

fritzdj
Copy link
Contributor

@fritzdj fritzdj commented Aug 30, 2023

We are using a logging integration for internal alerting and we want to be able to filter out certain error logs where the LD client is running into a "recoverable" error if possible. Today we noticed there were HTTP responses with a 500 status being returned when StreamingUpdateProcessor was running -- this looked to be a minor blip. Could we log these at the warning level instead of the error level if these are 'expected' blips?

@keelerm84
Copy link
Member

@fritzdj thank you for this contribution. These changes look good to me. Can you also update the behavior in the polling processor so the behavior stays consistent between the two data sources?

Thank you!

@fritzdj
Copy link
Contributor Author

fritzdj commented Sep 5, 2023

@fritzdj thank you for this contribution. These changes look good to me. Can you also update the behavior in the polling processor so the behavior stays consistent between the two data sources?

Thank you!

@keelerm84, thanks for the review! I just pushed the same change for PollingUpdateProcessor.

@fritzdj fritzdj changed the title StreamingUpdateProcessor logging update to warn on recoverable errors Processor logging updates to warn on recoverable errors Sep 5, 2023
@fritzdj fritzdj changed the title Processor logging updates to warn on recoverable errors Strean and polling processor logging updates to warn on recoverable errors Sep 5, 2023
@fritzdj fritzdj changed the title Strean and polling processor logging updates to warn on recoverable errors Stream and polling processor logging updates to warn on recoverable errors Sep 5, 2023
@keelerm84 keelerm84 changed the base branch from main to contrib September 6, 2023 13:05
Copy link
Member

@keelerm84 keelerm84 left a comment

Choose a reason for hiding this comment

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

Thank you for making that update! I will work on getting this released and update you here once it is done.

@keelerm84 keelerm84 merged commit 37e38e2 into launchdarkly:contrib Sep 6, 2023
LaunchDarklyReleaseBot pushed a commit that referenced this pull request Sep 6, 2023
Our documentation file had a configuration issue, which emits a warning
when building with sphinx. This caused the build process on
readthedocs.org to fail because they treat warnings as errors.

We didn't catch this because:

1. Warnings are treated like warnings when generating documentation
   locally.
2. We don't try to generate the documentation as part of our CI process.

This commit should resolve both the underlying issue and the
deficiencies that prevented us from catching this initially.
LaunchDarklyReleaseBot added a commit that referenced this pull request Sep 6, 2023
## [8.1.6] - 2023-09-06
### Changed:
- Recoverable errors are logged as a warning not an error. (Thanks,
[fritzdj](#219)!)

---------

Co-authored-by: Ben Woskow <48036130+bwoskow-ld@users.noreply.github.com>
Co-authored-by: Eli Bishop <eli@launchdarkly.com>
Co-authored-by: LaunchDarklyCI <dev@launchdarkly.com>
Co-authored-by: LaunchDarklyCI <LaunchDarklyCI@users.noreply.github.com>
Co-authored-by: hroederld <hroeder@launchdarkly.com>
Co-authored-by: Robert J. Neal <rneal@launchdarkly.com>
Co-authored-by: Robert J. Neal <robertjneal@users.noreply.github.com>
Co-authored-by: Ember Stevens <ember.stevens@launchdarkly.com>
Co-authored-by: ember-stevens <79482775+ember-stevens@users.noreply.github.com>
Co-authored-by: Matthew M. Keeler <keelerm84@gmail.com>
Co-authored-by: charukiewicz <charukiewicz@protonmail.com>
Co-authored-by: LaunchDarklyReleaseBot <launchdarklyreleasebot@launchdarkly.com>
Co-authored-by: Christian Charukiewicz <christian@foxhound.systems>
Co-authored-by: Matthew M. Keeler <mkeeler@launchdarkly.com>
Co-authored-by: Ben Woskow <bwoskow@launchdarkly.com>
Co-authored-by: Gavin Whelan <gwhelan@launchdarkly.com>
Co-authored-by: Elliot <35050275+Apache-HB@users.noreply.github.com>
Co-authored-by: Gabor Angeli <gabor@squareup.com>
Co-authored-by: Elliot <apachehaisley@gmail.com>
Co-authored-by: Louis Chan <lchan@launchdarkly.com>
Co-authored-by: prpnmac <95777763+prpnmac@users.noreply.github.com>
Co-authored-by: Louis Chan <91093020+louis-launchdarkly@users.noreply.github.com>
Co-authored-by: Daniel Fritz <dfritz@indigoag.com>
@keelerm84
Copy link
Member

Fixed in 8.1.6

And once again, thank you for your contribution!

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