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

[EP-2470] Enable DD APM Tracing #1075

Merged
merged 3 commits into from
Nov 15, 2023
Merged

[EP-2470] Enable DD APM Tracing #1075

merged 3 commits into from
Nov 15, 2023

Conversation

mattdrees
Copy link
Member

Jira

This configures the DD RUM component to send tracing headers in all requests sent to the api url
(cortex gateway). This should let us correlate front-end behavior with back-end traces/errors/logs.

This configures the DD RUM component to send tracing headers in all requests sent to the api url
(cortex gateway). This should let us correlate front-end behavior with back-end traces/errors/logs.

I am not %100 sure that envServiceProvider can have read() called like this; but I see other envService
methods being called on the provider, so I'm guessing it works.
I don't have a quick way to test this locally.
Probably left over copy/paste from the rollbar specs.
@mattdrees
Copy link
Member Author

@dr-bizz would you be up for helping me figure out the right way to get the 'apiUrl' param at config time? I made a guess here, but the unit tests don't like it and I think it's likely that it would break at runtime as well.

@dr-bizz
Copy link
Contributor

dr-bizz commented Nov 13, 2023

@dr-bizz would you be up for helping me figure out the right way to get the 'apiUrl' param at config time? I made a guess here, but the unit tests don't like it and I think it's likely that it would break at runtime as well.

What you've done is correct, let me take a look at the tests.

@dr-bizz
Copy link
Contributor

dr-bizz commented Nov 14, 2023

This fix will work.

@mattdrees
Copy link
Member Author

thanks @dr-bizz !

@wrandall22 wrandall22 self-requested a review November 14, 2023 20:02
@mattdrees
Copy link
Member Author

I'll merge (which I believe auto-deploys to prod) in the morning

@dr-bizz
Copy link
Contributor

dr-bizz commented Nov 15, 2023

Yes, merging does auto-deploy it to production

@wrandall22
Copy link
Contributor

Pushing to staging now so it can be tested there.

@wrandall22
Copy link
Contributor

Perhaps this shows traces like we mean to see them?

@mattdrees
Copy link
Member Author

Yep, that's what I was hoping to see!

@mattdrees mattdrees merged commit 8c585e3 into master Nov 15, 2023
5 checks passed
@mattdrees mattdrees deleted the enable-tracing-for-dd-rum branch November 15, 2023 17:20
@wrandall22
Copy link
Contributor

I wish errors like this one had traces on them. I wonder why they don't.

@wrandall22
Copy link
Contributor

Actually, it looks like the traces are attached to the parent action.

@mattdrees
Copy link
Member Author

Right.

I don't understand, though, why that error doesn't show up in the Errors page.

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