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

Fixing intermittent panic on metric submission #92

Merged
merged 1 commit into from
Feb 1, 2023

Conversation

Areson
Copy link
Collaborator

@Areson Areson commented Feb 1, 2023

In certain (unknown) cases, submitting metrics via the Datadog API fails with the error EOF and a nil http response. It appears that while the Datadog API config has a Compress field, setting it to true does not actually enable compression. It only prevents it from disabling compression:

if !c.Cfg.Compress {
	// gzip is on by default, so disable it by setting encoding to identity
	localVarRequest.Header.Add("Accept-Encoding", "identity")
}

Despite the Datadog client indicating the gzip compression is on by default, an inspection of the code and dumping the request indicates that it is not. Manually setting the content encoding on each request is required to actually enable compression.

In some case the lack of compression caused a failure in the underlying http client which resulted in an EOF error. It's unclear what payload size or conditions resulted in this issue, but adding the content encoding resolved the issue.

  • Added content encoding of gzip when compress is set to true
  • Added a nil check on the http response to prevent trying to get the status code from it when it is nil

In certain (unknown) cases, submitting metrics via the Datadog API fails with the error `EOF` and a `nil` http response. It appears that while the API config has a `compress` field, setting it to `true` does not actually enable compression. It only prevents it from disabling compression:
```golang
if !c.Cfg.Compress {
		// gzip is on by default, so disable it by setting encoding to identity
		localVarRequest.Header.Add("Accept-Encoding", "identity")
	}
```

Despite the Datadog client indicating the gzip compression is on by default, an inspection of the code and dumping the request indicates that it is not. Manually setting the content encoding on each request is required to actually enable compression.

In some case the lack of compression caused a failure in the underlying http client which resulted in an `EOF` error. It's unclear what payload size or conditions resulted in this issue, but adding the content encoding resolved the issue.

- Added content encoding of `gzip` when `compress` is set to `true`
- Added a `nil` check on the http response to prevent trying to get the status code from it when it is `nil`
@Areson Areson requested a review from daniel-nichter February 1, 2023 01:12
if _, r, err := s.metricsApi.SubmitMetrics(ddCtx, *datadogV2.NewMetricPayload(dp[rangeStart:rangeEnd]), *datadogV2.NewSubmitMetricsOptionalParameters()); err != nil {
if r.StatusCode == http.StatusRequestEntityTooLarge {
if r != nil && r.StatusCode == http.StatusRequestEntityTooLarge {
Copy link
Member

Choose a reason for hiding this comment

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

Yeah when err is nil, there's usually no guarantee on the response. This is how built-in http works, at least.

@Areson Areson merged commit d20319b into main Feb 1, 2023
@Areson Areson deleted the ioberst-dd-sink-panic branch February 1, 2023 16:21
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