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

[receiver/datadogreceiver] Return json for traces if the version is > 0.3 #35705

Conversation

jmeagher
Copy link
Contributor

@jmeagher jmeagher commented Oct 8, 2024

Description

Problem:

Fix: For trace submissions with a version > 0.3 return a valid, but empty Json response.

Testing

Added unit test to ensure the expected responses to traces are returned.

Copy link

linux-foundation-easycla bot commented Oct 8, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@MovieStoreGuy
Copy link
Contributor

WIP for now while I'm figuring out the code base

Is there anything you need help with please reach here or within the CNCF slack :)

@jmeagher jmeagher marked this pull request as ready for review October 9, 2024 20:29
@jmeagher jmeagher requested a review from a team as a code owner October 9, 2024 20:29
@jmeagher
Copy link
Contributor Author

jmeagher commented Oct 9, 2024

Thanks @MovieStoreGuy. It looks like I'll need a little assistance in getting the workflow to run. Also any other assistance to get this merged .

@jmeagher jmeagher changed the title WIP Return json for traces if the version is > 0.3 Return json for traces if the version is > 0.3 Oct 9, 2024
@jmeagher jmeagher changed the title Return json for traces if the version is > 0.3 [receiver/datadogreceiver] Return json for traces if the version is > 0.3 Oct 9, 2024
@jmeagher jmeagher force-pushed the jmeagher-return-json-for-some-traces branch from b276543 to c08a3c1 Compare October 10, 2024 00:34
@jmeagher
Copy link
Contributor Author

This is ready for a re-run of the tests. It got a failure on the last run from something Elasticsearch related. I rebased and hope it will pass this time. I didn't see any errors related to this code change.

@jmeagher jmeagher force-pushed the jmeagher-return-json-for-some-traces branch from c08a3c1 to 6d76385 Compare October 10, 2024 19:20
@jmeagher
Copy link
Contributor Author

I'm not authorized to merge this. If you're happy with it so am I. Merge at will.

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Oct 29, 2024
@jmeagher
Copy link
Contributor Author

@MovieStoreGuy @jpkrohling what are the next steps for this? I think it's all ready for a merge or any additional reviews that are needed.

@github-actions github-actions bot removed the Stale label Oct 30, 2024
@MovieStoreGuy MovieStoreGuy merged commit 41bc833 into open-telemetry:main Oct 30, 2024
158 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 30, 2024
ArthurSens pushed a commit to ArthurSens/opentelemetry-collector-contrib that referenced this pull request Nov 4, 2024
… 0.3 (open-telemetry#35705)

#### Description

Problem: 

* [The Datadog agent for versions > 0.3 returns Json instead of
"Ok"](https://github.com/DataDog/datadog-agent/blob/86b2ae24f93941447a5bf0a2b6419caed77e76dd/pkg/trace/api/api.go#L511-L519)
* [dd-trace-js is using V0.4 for sending data to Datadog and that
expects to get a JSON response
back](https://github.com/DataDog/dd-trace-js/blob/2d175d30d5ad7291c238819116a07f003074316b/packages/dd-trace/src/exporters/agent/writer.js#L52)
* When sending traces from NodeJS with the dd-trace-js library it works,
but throws a high volume of errors from the non-json response.

Fix: For trace submissions with a version > 0.3 return a valid, but
empty Json response.

<!--Describe what testing was performed and which tests were added.-->
#### Testing

Added unit test to ensure the expected responses to traces are returned.

---------

Co-authored-by: Sean Marciniak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants