Add "Show trace logs" action#34310
Conversation
af8bfe5 to
3c18f8a
Compare
💚 Build Succeeded |
|
Looking at this more closely, I'm not sure anything we're doing with time ranges for these links is actually working. Going to take a closer look at those and see if we can find a simple fix for that here if necessary. |
💚 Build Succeeded |
f677abf to
aaff150
Compare
💔 Build Failed |
10f8981 to
23c9677
Compare
23c9677 to
83497c6
Compare
💚 Build Succeeded |
weltenwort
left a comment
There was a problem hiding this comment.
The generated link looks good and seems to work with my test data. 👍
| ), | ||
| target: traceId, | ||
| hash: `/link-to/logs`, | ||
| query: { time, filter: `trace.id:${traceId}` } |
There was a problem hiding this comment.
I'm not seeing this here, but wanted to mention it because I got burned by this before:
Characters like : in the URL might be strictly encoded by the querystring module, which is seems to be used here. I have run into situations where this caused duplicate history entries due to angular undoing the encoding of certain characters. I previously prevented that by using the encoding functions provided by ui/utils/query_string instead of those from querystring.
There was a problem hiding this comment.
Thanks, we'll look into this -- we already handle this issue with the % character, but maybe we can consolidate into one solution.
* Adds trace logs link to action menu * Updates tests * Fixes time range values for action menu links * Relaxes type checking on query param object to allow for outgoing links * Revert unnecessary rison changes after realizing infra links do not accept rison
* Adds trace logs link to action menu * Updates tests * Fixes time range values for action menu links * Relaxes type checking on query param object to allow for outgoing links * Revert unnecessary rison changes after realizing infra links do not accept rison
Closes #23317
Note: we should separate link creation logic per Kibana destination, so we have better type safety around things like query params. In this PR I've relaxed the query param types significantly, but in a separate branch I'm going to try to make this better with destination-specific types (ML vs APM vs Discover etc) ... see also #34443
Also fixes a bug discovered while working on this ticket, where links to infra/logs were not including correct time params.
Summary
Adds a conditional "Show trace logs" link to the transaction action menu (if trace.id is present).
Also:
While working on this ticket, I also discovered that the links to infra were not including the correct time params (we were using rison-based params which infra does not recognize). This ticket fixes that by using the correctly formatted params for time for logs and metrics links.