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] Adds "Logs" tab to transaction details for related trace logs #85859

Merged
merged 1 commit into from
Dec 15, 2020

Conversation

ogupte
Copy link
Contributor

@ogupte ogupte commented Dec 14, 2020

Closes #79995 by adding new tab in transaction details to show related trace logs.

Screen Shot 2020-12-14 at 3 46 45 PM

@ogupte ogupte requested a review from a team as a code owner December 14, 2020 20:52
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Dec 14, 2020
@elasticmachine
Copy link
Contributor

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

<LogStream
startTimestamp={startTimestamp - framePaddingMs}
endTimestamp={endTimestamp + framePaddingMs}
query={`trace.id:"${transaction.trace.id}" OR "${transaction.trace.id}"`}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This matches the filter for the "Trace logs" link in the action menu: https://github.com/elastic/kibana/blob/master/x-pack/plugins/apm/public/components/shared/TransactionActionMenu/sections.ts#L183, but i'm not sure why this needs to be an OR, is it just me or is this redundant?

Copy link
Contributor

Choose a reason for hiding this comment

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

My guess is that is will pick up entries that don't have the trace.id attribute set but contain the trace id in other fields, but first matching with the field name gives exact matches.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
apm 5.4MB 5.4MB +1.2KB

Distributable file count

id before after diff
default 47129 47889 +760

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
apm 48.8KB 49.1KB +232.0B

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

@@ -46,9 +33,10 @@ export function TransactionTabs({
exceedsMax,
}: Props) {
const history = useHistory();
const tabs = [timelineTab, metadataTab];
const tabs = [timelineTab, metadataTab, logsTab];
Copy link
Member

@sorenlouv sorenlouv Dec 14, 2020

Choose a reason for hiding this comment

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

nit: might want to define this outside the component to indicate that it is static.

Copy link
Member

Choose a reason for hiding this comment

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

Nvm, I see a green build. Feel free to merge.

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.

thanks for getting this in so quickly! lgtm

@ogupte ogupte merged commit 6f05077 into elastic:master Dec 15, 2020
ogupte added a commit to ogupte/kibana that referenced this pull request Dec 15, 2020
ogupte added a commit that referenced this pull request Dec 15, 2020
…d trace logs. (#85859) (#85896)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@weltenwort
Copy link
Member

Just for awareness, #85148 introduced the ability to customize the rendering of the field contents. 😉

gmmorris added a commit to ymao1/kibana that referenced this pull request Dec 15, 2020
* master: (66 commits)
  [Alerting] fixes broken Alerting Example plugin (elastic#85774)
  [APM] Service overview instances table (elastic#85770)
  [Security Solution] Unskip timeline creation Cypress test (elastic#85871)
  properly recognize enterprise licenses (elastic#85849)
  [SecuritySolution][Detections] Adds SavedObject persistence to Signals Migrations (elastic#85690)
  [TSVB] Fix functional tests flakiness and unskip them (elastic#85388)
  [Fleet] Change permissions for Fleet enroll role (elastic#85802)
  Gauge visualization can no longer be clicked to filter on values since Kibana 7.10.0 (elastic#84768)
  [Security Solution][Detections] Add alert source to detection rule action context (elastic#85488)
  [Discover] Don't display hide/show button for histogram when there's no time filter (elastic#85424)
  skip flaky suite (elastic#78553)
  License checks for alerts plugin (elastic#85649)
  skip flaky suite (elastic#84992)
  skip 'query return results valid for scripted field' elastic#78553
  Allow action types to perform their own mustache variable escaping in parameter templates (elastic#83919)
  [ML] More machine learning links in doc_links_service.ts (elastic#85365)
  Removed Alerting & Event Log deprecated fields that should not be using (elastic#85652)
  Closes elastic#79995 by adding new tab in transaction details to show related trace logs. (elastic#85859)
  Fix outdated jest snapshot
  [Maps] Surface on prem EMS (elastic#85729)
  ...
@formgeist
Copy link
Contributor

Just for awareness, #85148 introduced the ability to customize the rendering of the field contents. 😉

Ah, good point! I've opened an issue to track configuring the columns - unsure what can be shown and if we should do this now (superseding the possible custom settings)

@cauemarcondes
Copy link
Contributor

Tests ok.
New bug opened #86962

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:test-plan-7.11.0 apm:test-plan-done Pull request that was successfully tested during the test plan Team:APM All issues that need APM UI Team support v7.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[APM] Display related trace logs
8 participants