Block trace on more query results #21826
Conversation
| React.useCallback(async () => { | ||
| await Promise.all([querySensor(), querySensorAssetSelection()]); |
There was a problem hiding this comment.
Does this particular bit do anything? Does the delayed row query have tracing built into it?
There was a problem hiding this comment.
ah no, forgot to remove this
| if (hasMore) { | ||
| startPolling(POLL_INTERVAL); | ||
| } else { | ||
| dependency.completeDependency(CompletionType.SUCCESS); |
There was a problem hiding this comment.
I don't think this is correct - it means that if a run is in progress and we start polling for logs here, we will not mark the page as loaded until the run completes?
| variables: {snapshotId}, | ||
| }); | ||
|
|
||
| const {data, loading} = queryResult; | ||
| useBlockTraceOnQueryResult(queryResult, 'SnapshotQuery'); |
There was a problem hiding this comment.
It seems like most of the time, if you skip this line, you are messing up the datadog traces...
I think it'd be worth evaluating whether a different design would make this less fragile - wrapping useQuery itself like we talked about, or making our three <Loading /> components block the trace, etc.
Just seems like this will almost certainly be broken in subtle ways 12mo from now unless we're vigilant about testing the datadog output of new pages. If we need to do it this way, maybe we could add a visual indicator on the page in dev that shows you what the datadog trace status is. (eg: an actual dot that goes from red to green when the dd trace is loaded, so you can verify it matches your loading states)
There was a problem hiding this comment.
Just seems like this will almost certainly be broken in subtle ways 12mo from now unless we're vigilant about testing the datadog output of new pages. If we need to do it this way, maybe we could add a visual indicator on the page in dev that shows you what the datadog trace status is. (eg: an actual dot that goes from red to green when the dd trace is loaded, so you can verify it matches your loading states)
Yeah 100% agree. I think a visual indicator is a great idea.
Summary & Motivation
Was looking at traces and saw that I missed a spot, then realized I missed a bunch more, adding those too.
How I Tested These Changes
Took traces, eg: https://app.datadoghq.com/rum/sessions?query=%40application.id%3A6d12cca2-0263-463b-a90e-cc6c524e14bd%20%40usr.email%3Amarco%40dagsterlabs.com%20%40type%3Aview&agg_m=count&agg_m_source=base&agg_q=%40issue.id&agg_q_source=base&agg_t=count&cols=&event=AgAAAY90cI-CjM0zYQAAAAAAAAAYAAAAAEFZOTBjSS1DQUFBVktEWTBTSGZtdzJNWAAAACQAAAAAMDE4Zjc0NzAtOGY4Mi00ZmMwLWE2ZTUtMjczMjczYTRjYzA4&fromUser=false&tab=error&top_n=10&top_o=top&viz=stream&x_missing=true&from_ts=1715645287234&to_ts=1715645587234&live=true