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

[APM] Add link-to/transaction route #101731

Merged
merged 1 commit into from
Jun 9, 2021

Conversation

dgieselaar
Copy link
Member

Adds a /app/apm/link-to/transaction/:transactionId route, that will link to a transaction detail page for the transaction, instead of the root transaction of a trace.

I've used this in demo's, for linking from a transaction in discover to the trace waterfall. Not sure if this is useful enough, but the maintenance costs seems low, so just putting it up for discussion.

@dgieselaar dgieselaar added Team:APM All issues that need APM UI Team support release_note:skip Skip the PR/issue when compiling release notes v7.14.0 labels Jun 9, 2021
@dgieselaar dgieselaar requested a review from a team as a code owner June 9, 2021 11:02
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

lgtm. transaction.id is not guaranteed to be unique outside of a trace but that's more of a theoretical problem, and probably not something we care about in this case.

{ term: { [TRACE_ID]: traceId } },
...rangeQuery(start, end),
...(traceId ? [{ term: { [TRACE_ID]: traceId } }] : []),
...('start' in setup ? rangeQuery(setup.start, setup.end) : []),
Copy link
Member

Choose a reason for hiding this comment

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

Are setup.start and setup.end typed as optionals here? Would be great if they were but I seem to recall we require them on every api

Copy link
Member

Choose a reason for hiding this comment

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

(or maybe it's the opposite problem I'm thinking about where they are always optional, even when they are required on the api)

Copy link
Member Author

Choose a reason for hiding this comment

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

This function is used in different endpoints, one that does have a start/end query parameter, and one that doesn't. The type for setup here is a union type of setup without start/end (not defined) and setup with start end (always defined).

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1599 1600 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 4.3MB 4.3MB +1.3KB

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@dgieselaar dgieselaar added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 9, 2021
@dgieselaar dgieselaar merged commit 520d1e8 into elastic:master Jun 9, 2021
@dgieselaar dgieselaar deleted the link-to-transaction branch June 9, 2021 13:42
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 9, 2021
kibanamachine added a commit that referenced this pull request Jun 9, 2021
Co-authored-by: Dario Gieselaar <dario.gieselaar@elastic.co>
<CentralizedContainer>
<EuiEmptyPrompt
iconType="apmTrace"
title={<h2>Fetching transaction...</h2>}
Copy link
Contributor

Choose a reason for hiding this comment

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

i18n this?

gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 9, 2021
* master:
  clarify which parts of TM are experimental (elastic#101757)
  Add sh scripts with _bulk_action route usage examples (elastic#101736)
  [Uptime] Only register route in side nav if uptime show capability is true (elastic#101709)
  Use KIBANA_DOCS in doc link service (elastic#101667)
  [Alerting][Event log] Persisting duration information for active alerts in event log (elastic#101387)
  Address design issues in Discover/Graph (elastic#101584)
  Optimize performance for document table (elastic#101715)
  Change file data visualizer links to point to new location in home application (elastic#101393)
  [Fleet] Tighten policy permissions, take II (elastic#97366)
  [ML] Add debounce to the severity control update  (elastic#101581)
  [Fleet] Fix routing issues with `getPath` and `history.push` (elastic#101658)
  [APM] Add link-to/transaction route (elastic#101731)
  [Index Patterns] Runtime fields CRUD REST API  (elastic#101164)
  [ILM] Refactor types and fix missing aria labels (elastic#101518)
  [Lens] New summary row feature for datatable (elastic#101075)
  Blocks save event filter with a white space name (elastic#101599)
  Improve security server types (elastic#101661)
  [APM] Replace side nav with tabs on Settings page (elastic#101460)
  [APM] Only register items in side nav if user has permissions to see app (elastic#101707)
  [Security solution][Endpoint] Add back button when to the event filters list (elastic#101280)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v7.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants