feat(explore): Wire up data fetching hooks for cross event queries #104233
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #104233 +/- ##
===========================================
- Coverage 80.52% 80.52% -0.01%
===========================================
Files 9345 9345
Lines 399870 399867 -3
Branches 25644 25643 -1
===========================================
- Hits 321999 321996 -3
Misses 77423 77423
Partials 448 448 |
|
@sentry review |
3a62553 to
50c20a5
Compare
|
@sentry review |
|
@sentry review |
gggritso
left a comment
There was a problem hiding this comment.
👀 the change to useFetchEventsTimeSeries is pretty confusing, I don't think it fits well with how the hook operates. Is it possible to clarify? I left a specific comment on the file. No feelings one way or the other about the rest of it!
gggritso
left a comment
There was a problem hiding this comment.
Makes sense, I think! I bet that someone who knows this feature better would have better comments, but there are a few things that stuck out to me
| const logQuery: string[] = []; | ||
| const metricQuery: string[] = []; | ||
| const spanQuery: string[] = []; | ||
|
|
||
| for (const crossEvent of slicedCrossEvents) { | ||
| switch (crossEvent.type) { | ||
| case 'spans': | ||
| spanQuery.push(crossEvent.query); | ||
| break; | ||
| case 'logs': | ||
| logQuery.push(crossEvent.query); | ||
| break; | ||
| case 'metrics': | ||
| metricQuery.push(crossEvent.query); | ||
| break; | ||
| default: | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
This is kind of a bummer, IMO having to massage parameters that we control is an anti-pattern. Cross event info is stored as JSON in the URL, is it not? if yes, can it be stored in the safe format that it's passed around the codebase?
There was a problem hiding this comment.
As an aside this seems like it'd work great with Nuqs parsers
There was a problem hiding this comment.
If you use the useQueryParamsCrossEvents() it returns an array of cross event objects, this is just a util hook to tidy up things for passing down to data fetching hooks.
useQueryParamsCrossEvents is built into the wider explore query params context setup, so I don't know how easy/hard it would be to convert that over to nuqs.
899d596 to
9c50fc4
Compare
This pull request introduces support for cross-event querying in several hooks and components, enabling the ability to filter and query across logs, metrics, and spans. The main changes include the addition of a new hook for handling cross-event queries, updates to options and query parameter interfaces to accept new query types, and propagation of these queries throughout the codebase.
Ticket: EXP-621
useCrossEventQuerieshook along with tests, enabling extraction and organization of queries for logs, metrics, and spans from context, with safeguards for maximum query limits and type validation.logQuery,metricQuery, andspanQueryarrays, ensuring these queries can be propagated to API requests.