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

[poc]: revert rum injection poc on master for now #1167

Merged
merged 2 commits into from
Sep 4, 2020

Conversation

ericmustin
Copy link
Contributor

Pulling this off master for now and keeping it on a feature branch until we have more confidence around it cc @marcotc @brettlangdon

@ericmustin ericmustin requested a review from a team September 4, 2020 11:47
brettlangdon
brettlangdon previously approved these changes Sep 4, 2020
@ericmustin ericmustin self-assigned this Sep 4, 2020
@ericmustin ericmustin added this to the 0.40.0 milestone Sep 4, 2020
Copy link
Member

@marcotc marcotc left a comment

Choose a reason for hiding this comment

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

👍

@ericmustin ericmustin merged commit d4f311d into master Sep 4, 2020
@ivoanjo ivoanjo deleted the revert_rum_injection branch July 16, 2021 09:13
@ryush00
Copy link

ryush00 commented Aug 29, 2023

Hello there. I found this PR through a search and would like to know when the feature will be back. What was the background to revert?

DataDog/browser-sdk#492

@ericmustin
Copy link
Contributor Author

@ryush00 hey, I don't work at ddog anymore but I think the context at the time was that there was some issues with implementing this across languages (while ruby/rack has some wonderful support for this sortve thing, other langs like node et al did not), and also we didn't have confidence that, in practice, caching at proxy layers like a CDN wouldn't create issues/incorrect trace-Id associations btwn client and server.

This was +3yrs ago tho so I may be mistaken and the state of the art may have changed, but just sharing that context in the hopes that it's helpful.

@ryush00
Copy link

ryush00 commented Aug 29, 2023

I really appreciate you sharing the context, Eric. It's very helpful. Have a great day. 😄

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.

4 participants