Might want to be more specific about valid statuses here. Pretty sure the agent would only normally send a 200, and anything else would be surprising.

The source code agrees with you: https://github.com/DataDog/datadog-agent/blob/main/pkg/trace/api/api.go

However, the status is not part of the documented contract between the agent and tracer libraries (what little contract there is: I did some documentation here).

I don't know how likely the de facto "only 200" is to change (probably 0%), but let's accept all of the 2xx anyway.

The java tracer checks [200, 300) as we do here. References:

• https://github.com/DataDog/dd-trace-java/blob/b25b4321c847f4206ab4970c8b6a9ab4f35ee838/dd-trace-core/src/main/java/datadog/trace/common/writer/ddintake/DDEvpProxyApi.java#LL124C6-L124C6
• https://square.github.io/okhttp/4.x/okhttp/okhttp3/-response/is-successful/

I'll leave it.

edit: One of the links above is for the Intake writer, not the Agent writer. I'm having trouble finding the corresponding logic in the Agent writer. Also, Doug claims that the Agent will sometimes return 200 when it is overloaded. Perhaps that's what was going on in the issue that inspired this pull request. I'll look into it more. Regardless, this logging could be valuable.

edit: I'll be damned, they do check only 200: https://github.com/DataDog/dd-trace-java/blob/b25b4321c847f4206ab4970c8b6a9ab4f35ee838/dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java#L116-L122. I'll change it here and make a note to change it in dd-trace-cpp as well.

The plot thickens: https://github.com/DataDog/datadog-agent/blob/3723c72a732fb26510e4813690d2cc92e2732f2a/pkg/trace/api/api.go#L89-L92

Since the Agent might return 200 without a response body in cases that should be 429 "too many requests," now I'm wondering if we should ignore empty response bodies in all cases.

edit: In fact, it will still return the sample rates JSON body even when overloaded.

The question now is whether the Agent will ever return an empty response body in a truly success-indicating response.

It will not. Here is what I will implement:

David Goffredo
 Here's my plan:
 - transport issue: log error
 - non-200 response: log error
 - 200 response without body: log error
 - 200 response with body: assume JSON

 yeah?

Kyle Nusbaum
 Sounds good.
 I would distinguish between totally empty body and an empty json object. {} seems like a valid response.

David Goffredo
 correct

Kyle Nusbaum
 Then looks good.

David Goffredo
 by "empty," I mean length zero

 coolio, thanks a third time

Kyle Nusbaum
 No problem.

I suspect the body would be empty, but if it weren't, it could be pretty long.
So maybe just the first 100 bytes or so instead of the entire body?

I considered that. I agree that the response will most likely be empty; but if it's not empty, I don't think it will be long. What might it contain? In the 200 case, it's a JSON object containing one entry per root service seen recently by the Agent, but that's not relevant here. In the error case, it might be... a stack trace? This is Go we're talking about. Logging the body is kind of silly, but I don't think that excessive length will be a problem. Let's leave it.

not -> no