Skip to content

Conversation

@pulpdrew
Copy link
Contributor

@pulpdrew pulpdrew commented Nov 18, 2025

Closes HDX-2776
Closes HDX-2752

Summary

This PR extends the custom attributes feature from #1356 to support custom attributes on individual rows (rather than entire traces). Further, attributes which are URLs are now rendered as links, supporting use-cases which require building custom external links from queried data.

Demo

Source Configuration:

Screenshot 2025-11-18 at 6 01 55 PM

Side panel:
Screenshot 2025-11-18 at 6 02 12 PM

Inferred link:
Screenshot 2025-11-18 at 6 02 32 PM

logview attribute definition
if(
  NOT empty(TraceId) AND NOT empty(SpanId) AND NOT empty(ServiceName),
  concat('https://logview.com?q=',
    'trace_id=', TraceId,
    '&span_id=', SpanId,
    '&service=', ServiceName,
    '&start_time=', formatDateTime(Timestamp - INTERVAL 1 SECOND, '%FT%T.%fZ'),
    '&end_time=', formatDateTime(Timestamp + Duration/1e9 + INTERVAL 1 SECOND, '%FT%T.%fZ')
  ),
  ''
)

@changeset-bot
Copy link

changeset-bot bot commented Nov 18, 2025

🦋 Changeset detected

Latest commit: 2d5bd7a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/common-utils Patch
@hyperdx/app Patch
@hyperdx/api Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Nov 18, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview Comment Nov 19, 2025 9:37pm

@claude
Copy link

claude bot commented Nov 18, 2025

Code Review

Overall Assessment: No critical issues found. The PR implements row-level custom attributes cleanly with good test coverage.

Minor Observations:

  • ⚠️ Type safety: EventTag.tsx now accepts value: string | unknown but only handles string values in the link detection logic. Consider adding runtime type checking or constraining the type.

  • 📝 Migration consideration: The new highlightedRowAttributeExpressions field in MongoDB has no migration script. Existing sources will have undefined for this field (which is handled correctly with optional chaining).

  • 🧪 Test coverage: Excellent comprehensive tests for isLinkableUrl covering XSS vectors. No security concerns detected.

Positive Highlights:

  • ✅ Proper XSS prevention with protocol validation (http:/https: only)
  • ✅ Backward compatibility maintained (service name expression still works)
  • ✅ Clean separation between row-level and trace-level attributes
  • ✅ Consistent with existing patterns (follows highlightedTraceAttributeExpressions model)
  • ✅ 284 new test cases with thorough edge case coverage

Recommendation: ✅ Approve and merge

@github-actions
Copy link
Contributor

github-actions bot commented Nov 18, 2025

E2E Test Results

All tests passed • 38 passed • 3 skipped • 413s

Status Count
✅ Passed 38
❌ Failed 0
⚠️ Flaky 2
⏭️ Skipped 3

View full report →

@pulpdrew pulpdrew requested review from a team and brandon-pereira and removed request for a team November 19, 2025 14:25
@brandon-pereira
Copy link
Member

We should fix the wrapping here. I assume the hover to see the link is potentially security related - so this defeats it.

Screenshot 2025-11-19 at 2 05 50 PM

This could be intended, not sure, but if I put the inferred link on the highlighted trace attributes, I was expecting to only see the actively highlighted link not all links.

Screenshot 2025-11-19 at 2 05 44 PM

@pulpdrew
Copy link
Contributor Author

This could be intended, not sure, but if I put the inferred link on the highlighted trace attributes, I was expecting to only see the actively highlighted link not all links.

That is intended - the trace attributes are shown for any spans they're found in. This is for a use-case where some span in a trace has a particular property of interest, and you don't want to have to figure out which span it's in by clicking them.

This does potentially point towards a need to show the row-level attributes for the selected waterfall span somewhere as well, in the trace panel, but I'd prefer to handle that as an enhancement if that's OK with you!

@pulpdrew
Copy link
Contributor Author

We should fix the wrapping here. I assume the hover to see the link is potentially security related - so this defeats it.

Great catch! Updated the word break so things wrap better now

Screenshot 2025-11-19 at 4 27 59 PM

@kodiakhq kodiakhq bot merged commit 59422a1 into main Nov 19, 2025
9 checks passed
@kodiakhq kodiakhq bot deleted the drew/single-row-attributes branch November 19, 2025 21:38
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