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

fix: add case for error http status #8

Merged
merged 2 commits into from
Mar 30, 2023
Merged

fix: add case for error http status #8

merged 2 commits into from
Mar 30, 2023

Conversation

btkostner
Copy link
Contributor

@btkostner btkostner commented Mar 30, 2023

This should prevent a raised error like such:

(CaseClauseError no case clause matching: {:ok, %Finch.Response{body: "", headers: [{"datadog-agent-state", "3e08d91eabb10dd5b61fe89c78d84597c33b237eea8bed02ca3e8193ff1beac8"}, {"datadog-agent-version", "7.41.1"}], status: 502}})

Instead, it will catch the error and run Logger.error("Error sending metrics to Datadog", error: error). While still noisy in logs, it should not show up in error reporters like Sentry or DataDog.

@btkostner btkostner requested a review from a team March 30, 2023 17:40
@btkostner btkostner self-assigned this Mar 30, 2023
@btkostner btkostner requested a review from a team as a code owner March 30, 2023 17:40
doomspork
doomspork previously approved these changes Mar 30, 2023
Copy link

@doomspork doomspork left a comment

Choose a reason for hiding this comment

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

Thanks for the quick turn around! Wonder if we should be trying to capture something more on these errors so we can diagnose the cause if needed?

Edit: I can't approve 😥

@btkostner
Copy link
Contributor Author

btkostner commented Mar 30, 2023

So, we do log the error, though that's just text with the HTTP status. I guess we could log the whole thing though...

Update: It should log the whole response as metadata now.

@btkostner btkostner merged commit ef4a95d into main Mar 30, 2023
@btkostner btkostner deleted the btkostner-patch-1 branch March 30, 2023 17:46
This was referenced Mar 30, 2023
btkostner pushed a commit that referenced this pull request Apr 24, 2023
🤖 I have created a release *beep* *boop*
---


## 1.0.0 (2023-04-06)


### ⚠ BREAKING CHANGES

* Package and application configuration is now under `data_streams`
instead of `dd_data_streams`

### Features

* add basic implementation of ddsketch
([#1](#1))
([125b5ed](125b5ed))
* add basic kafka tracking support with data streams
([#3](#3))
([bfc6a0b](bfc6a0b))
* add LICENSE file
([#11](#11))
([6c5668f](6c5668f))
* link open telemetry span to current pathway context
([#5](#5))
([e0ed9b2](e0ed9b2))
* rename dd_data_streams to data_streams
([#9](#9))
([a0d1742](a0d1742))


### Bug Fixes

* add case for error http status
([#8](#8))
([ef4a95d](ef4a95d))
* **ci:** update PR title regex check
([64ef99f](64ef99f))
* dialyzer warnings for kafka integration map
([18bf936](18bf936))
* filter out nil values from kafka integration tags
([b33926f](b33926f))
* update kafka integration to not set context on produce
([#7](#7))
([6807b6d](6807b6d))
* update otel resource service configuration
([adb9890](adb9890))
* update tag logic to be more consistant
([#4](#4))
([48d13df](48d13df))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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.

3 participants