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

feat(explore): Linking to spans in traceview from all tables #78984

Merged
merged 4 commits into from
Oct 15, 2024

Conversation

Abdkhan14
Copy link
Contributor

@Abdkhan14 Abdkhan14 commented Oct 10, 2024

There's no concept of transaction.id in eap. We now have access to transaction.span_id in both EAP and Indexed datasets.

This PR enables sending transaction.span_id as targetId to the events_trace endpoint to ensure the transaction is always included in the response. Not the most proud of this solution, will follow up with PRs to completely replace eventId=transaction.id with targetId=transaction.span_id.

PR also enables finding transactions by their span_ids in the trace view. With this change we can successfully scroll to spans and txns in the traceview from all explore tables using only span_ids.

@Abdkhan14 Abdkhan14 requested review from a team as code owners October 10, 2024 23:58
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Oct 10, 2024
Copy link

codecov bot commented Oct 11, 2024

Bundle Report

Changes will increase total bundle size by 7.55kB (0.02%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
app-webpack-bundle-array-push 30.97MB 7.55kB (0.02%) ⬆️

Copy link

codecov bot commented Oct 11, 2024

❌ 2 Tests Failed:

Tests completed Failed Passed Skipped
7991 2 7989 0
View the top 2 failed tests by shortest run time
FieldRenderer tests renders span id link to traceview FieldRenderer tests renders span id link to traceview
Stack Traces | 0.023s run time
Error: expect(element).toHaveAttribute("href", ".../performance/trace/traceId/?node=span-spanId&node=txn-transactionSpanId&source=traces&statsPeriod=14d&targetId=transactionSpanId&timestamp=1727964900") // element.getAttribute("href") === ".../performance/trace/traceId/?node=span-spanId&node=txn-transactionSpanId&source=traces&statsPeriod=14d&targetId=transactionSpanId&timestamp=1727964900"

Expected the element to have attribute:
  href=".../performance/trace/traceId/?node=span-spanId&node=txn-transactionSpanId&source=traces&statsPeriod=14d&targetId=transactionSpanId&timestamp=1727964900"
Received:
  href=".../performance/trace/traceId/?eventId=transactionSpanId&node=span-spanId&node=txn-transactionSpanId&source=traces&statsPeriod=14d&targetId=transactionSpanId&timestamp=1727964900"
    at Object.<anonymous> (.../explore/tables/fieldRenderer.spec.tsx:56:59)
    at Promise.then.completed (.../jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../jest-circus/build/utils.js:231:10)
    at _callCircusTest (.../jest-circus/build/run.js:316:40)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at _runTest (.../jest-circus/build/run.js:252:3)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at run (.../jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (.../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)
FieldRenderer tests renders transaction id link to traceview FieldRenderer tests renders transaction id link to traceview
Stack Traces | 0.029s run time
Error: expect(element).toHaveAttribute("href", ".../performance/trace/traceId/?source=traces&statsPeriod=14d&targetId=transactionSpanId&timestamp=1727964900") // element.getAttribute("href") === ".../performance/trace/traceId/?source=traces&statsPeriod=14d&targetId=transactionSpanId&timestamp=1727964900"

Expected the element to have attribute:
  href=".../performance/trace/traceId/?source=traces&statsPeriod=14d&targetId=transactionSpanId&timestamp=1727964900"
Received:
  href=".../performance/trace/traceId/?eventId=transactionSpanId&source=traces&statsPeriod=14d&targetId=transactionSpanId&timestamp=1727964900"
    at Object.<anonymous> (.../explore/tables/fieldRenderer.spec.tsx:67:59)
    at Promise.then.completed (.../jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../jest-circus/build/utils.js:231:10)
    at _callCircusTest (.../jest-circus/build/run.js:316:40)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at _runTest (.../jest-circus/build/run.js:252:3)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at run (.../jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (.../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

Copy link
Member

@JonasBa JonasBa left a comment

Choose a reason for hiding this comment

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

Lets make sure old links still work and keep a fallback to eventId which we can probably fully remove this in a couple of weeks.

@Abdkhan14 Abdkhan14 requested a review from JonasBa October 11, 2024 15:27
@Abdkhan14 Abdkhan14 merged commit 78e8f3a into master Oct 15, 2024
43 checks passed
@Abdkhan14 Abdkhan14 deleted the abdk/explore-targetId branch October 15, 2024 14:46
cmanallen pushed a commit that referenced this pull request Oct 23, 2024
There's no concept of `transaction.id` in eap. We now have access to
`transaction.span_id` in both EAP and Indexed datasets.

This PR enables sending `transaction.span_id` as `targetId` to the
`events_trace` endpoint to ensure the transaction is always included in
the response. Not the most proud of this solution, will follow up with
PRs to completely replace `eventId`=`transaction.id` with
`targetId`=`transaction.span_id`.

PR also enables finding transactions by their span_ids in the trace
view. With this change we can successfully scroll to spans and txns in
the traceview from all explore tables using only span_ids.

---------

Co-authored-by: Abdullah Khan <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Oct 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants