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

[Maps] observability real user monitoring solution layer #64949

Merged
merged 27 commits into from
May 5, 2020

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Apr 30, 2020

Implements part of #64445.

This PR adds Observability layer wizard card making it easy to create map layers from Observability data. This PR provides two basic layer types for RUM transactions. Expect a lot more in this space in future PRs.

The card is only displayed when APM index pattern is installed. The APM index pattern is automatically installed when opening the APM application with data in apm-* index.

Screen Shot 2020-04-30 at 12 42 29 PM

Screen Shot 2020-04-30 at 12 42 40 PM

@nreese nreese added release_note:enhancement [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.8.0 labels Apr 30, 2020
@nreese nreese requested review from a team as code owners April 30, 2020 18:46
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@nreese nreese added the Team:APM All issues that need APM UI Team support label Apr 30, 2020
@elasticmachine
Copy link
Contributor

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

@nreese nreese requested a review from thomasneirynck May 4, 2020 19:48
@nreese
Copy link
Contributor Author

nreese commented May 4, 2020

@jsanz

The default green does not match very well with any of the ramps used, can I advocate for a darker grey instead? something like #3d3d3d?

done

Not really sure about this one because I see more cases where I don't like it, but for the APM RUM Performance -> SLA percentage cluster option, labeling would be nice

Added

@nreese
Copy link
Contributor Author

nreese commented May 4, 2020

@thomasneirynck

+1 on renaming some things. e.g.
"Observabiliby layers" -> "APM layers"
"Choropleth" -> "World boundaries"

done

@nreese
Copy link
Contributor Author

nreese commented May 4, 2020

@alexfrancoeur

For the performance layers I'm wondering if we should style green to red (fastest to slowest).

done

@nreese
Copy link
Contributor Author

nreese commented May 4, 2020

@elasticmachine merge upstream

Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

lgtm for Maps-code. Add-layer flow works really well, and good concept to start using for other "curated layers".


Main feedback is on the actual data-display side.

Use proper formatting of values

RUM Performance > SLA Percentage layer.

Format the % fields as actual percentages / trim decimal places

When these percentage values are shown as decimals, it looks funny on the map and the legend.

e.g.: This is how duration_sla_pct field formatter would look like
image

Map-formatting looks "better" imho.

image

Rum Performance > Transaction Duration layer.

Use duration formatter.
e.g.
image

This would format ms properly

image

@sqren : Is it possible to specify these formatters in the APM index pattern? The Maps-application will automatically pick these up to display values/format legends/tooltips etc...

Use custom ramps

It seems the performance-metrics are diverging. Presumably, a decrease is "better" (smaller than 0), and an increase is worse (larger than 0) (??) . Ideally, can we center the ramp at value 0 then. This would require a custom ramp iso using an out-of-the-box one.

@sqren Can you confirm if this is the case? Are the metrics for "transaction duration" and "SLA percentage" diverging? (either below or above 0.0)?

cc @alexfrancoeur Are the above suggestions required?

@sorenlouv
Copy link
Member

Is it possible to specify these formatters in the APM index pattern? The Maps-application will automatically pick these up to display values/format legends/tooltips etc...

@thomasneirynck

I think so - but since the APM app doesn't use index patterns I'm not the most knowledgable about this.
My understanding is that the index pattern is created by beats via the APM Server, and then semi-automatically added to kibana. Example: #64232
There might be a place in the APM Server settings where these formatters can be added.
@elastic/apm-server might know more (or know who to ask).

@thomasneirynck
Copy link
Contributor

thomasneirynck commented May 5, 2020

Seems like this would be the place to add a field-formatter to the apm-* index pattern (?)

"fieldFormatMap": "{\"client.bytes\":{\"id\":\"bytes\"},\"client.nat.port\":{\"id\":\"string\"},\"client.port\":{\"id\":\"string\"},\"destination.bytes\":{\"id\":\"bytes\"},\"destination.nat.port\":{\"id\":\"string\"},\"destination.port\":{\"id\":\"string\"},\"event.duration\":{\"id\":\"duration\",\"params\":{\"inputFormat\":\"nanoseconds\",\"outputFormat\":\"asMilliseconds\",\"outputPrecision\":1}},\"event.sequence\":{\"id\":\"string\"},\"event.severity\":{\"id\":\"string\"},\"http.request.body.bytes\":{\"id\":\"bytes\"},\"http.request.bytes\":{\"id\":\"bytes\"},\"http.response.body.bytes\":{\"id\":\"bytes\"},\"http.response.bytes\":{\"id\":\"bytes\"},\"http.response.status_code\":{\"id\":\"string\"},\"log.syslog.facility.code\":{\"id\":\"string\"},\"log.syslog.priority\":{\"id\":\"string\"},\"network.bytes\":{\"id\":\"bytes\"},\"package.size\":{\"id\":\"string\"},\"process.parent.pgid\":{\"id\":\"string\"},\"process.parent.pid\":{\"id\":\"string\"},\"process.parent.ppid\":{\"id\":\"string\"},\"process.parent.thread.id\":{\"id\":\"string\"},\"process.pgid\":{\"id\":\"string\"},\"process.pid\":{\"id\":\"string\"},\"process.ppid\":{\"id\":\"string\"},\"process.thread.id\":{\"id\":\"string\"},\"server.bytes\":{\"id\":\"bytes\"},\"server.nat.port\":{\"id\":\"string\"},\"server.port\":{\"id\":\"string\"},\"source.bytes\":{\"id\":\"bytes\"},\"source.nat.port\":{\"id\":\"string\"},\"source.port\":{\"id\":\"string\"},\"system.cpu.total.norm.pct\":{\"id\":\"percent\"},\"system.memory.actual.free\":{\"id\":\"bytes\"},\"system.memory.total\":{\"id\":\"bytes\"},\"system.process.cpu.total.norm.pct\":{\"id\":\"percent\"},\"system.process.memory.rss.bytes\":{\"id\":\"bytes\"},\"system.process.memory.size\":{\"id\":\"bytes\"},\"url.port\":{\"id\":\"string\"},\"view spans\":{\"id\":\"url\",\"params\":{\"labelTemplate\":\"View Spans\"}}}",

Would this impact any of the existing code @graphaelli (?)

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@miukimiu miukimiu self-requested a review May 5, 2020 13:51
@sorenlouv
Copy link
Member

sorenlouv commented May 5, 2020

Seems like this would be the place to add a field-formatter to the apm-* index pattern (?)

That thing is auto-generated, so adding anything there would be overwritten next time apm-server updates the index pattern.

@sorenlouv
Copy link
Member

sorenlouv commented May 5, 2020

Tangent: This shows why index pattern in the current form is such a bad fit. Having somewhere (like index patterns) to store field formatters only relevant to the UI, and having that live in Kibana, is a good idea. But persisting the field mappings in a secondary location (away from the actual mappings) is a terrible idea.
Since the index pattern now needs to be updated every single time a field in APM Server is added/updated we run into this problem where we need to merge two different responsibilities into one giant object.

Moving field formatters out of index patterns (or removing mappings from index patterns) would be a huge step forward, and resolve a lot of issues imo.

@thomasneirynck
Copy link
Contributor

@sqren 100% agreed

the functionality captured by index-patterns is really useful, but having IPs stored in a central location creates friction for use-cases like this. I think the kibana-app team is thinking in a similar direction in how they want to evolve e.g. Lens as well. cc @timroes

@alexfrancoeur
Copy link

@thomasneirynck I agree that a better long term solution be metric specific field formatters. Similar to Lens, Canvas and TSVB. It's a common request that we don't want to rely on global formatters for visualizing data. I'll add as a topic for discussion to our 7.9 planning. It's a feature I've wanted for nearly every map I build, and I know I'm not alone. For this PR, maybe we merge without addressing formatting initially?

@thomasneirynck
Copy link
Contributor

For this PR, maybe we merge without addressing formatting initially?

+1

Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

+1 maps-side.

There seems to be a little uncertainty about this sla_duration_pct field, where exactly this comes from. I would change the color-ramp to a custom diverging color-ramp that centers on 0. Depending on whether we get clarification on this field, we could consider this a bug fix and merge after FF. It seems that below 0 would indicate beating "sla", above 0 not meeting the SLA (?)

cc @alexfrancoeur

@simitt
Copy link
Contributor

simitt commented May 5, 2020

@thomasneirynck the index pattern is generated in APM Server via some libbeat functionality, and then copied over to Kibana to ensure the bundled index pattern is aligned with the APM Server index templates. The correct place to update any kind of formatting etc would be directly in APM Server, e.g. in the metricsets fields.yml.

Do you have a list of fields and formats for which the format should be changed?

@thomasneirynck
Copy link
Contributor

hi @simitt

two additions would be useful imho

Field: duration_sla_pct
Formatter: Percentage
Numeral js format pattern: 0,0.[]%

Field: transaction_duration_us
Formatter: Duration
Input format: Microseconds
Output format: Milliseconds
Decimal places: 2

I guess details can be tweaked, but those two fields are used in the solution-layers. It would make the labels on the map and the legend more readable.

I can create a separate issue for this as well, if that's easier. Thx!

@gchaps
Copy link
Contributor

gchaps commented May 5, 2020

I have a suggestion for the text underneath the Format field:

Formats allow you to control the display of specific values. Formats can also change the value and turn off highlighting in Discover.

If these are just "numeric" values, use that word instead of specific in the first sentence. Also in the second sentence, what does "change the value" mean? Change the value of the data?

@nreese nreese merged commit 891e27e into elastic:master May 5, 2020
nreese added a commit to nreese/kibana that referenced this pull request May 5, 2020
* [Maps] observability real user monitoring solution layer

* make stateful component

* rename RUM to observability

* layer select

* metrics select

* display select

* clean up

* create cholopleth map layer descriptor

* get styling working

* fix unique count agg

* create geo grid source

* render as heatmap

* clusters

* tslint errors

* last tslint fix

* only show card when APM index exists

* layer label

* clean up

* fix getLayerWizards

* add parans

* review feedback

* add cluster labels

* use green to red ramp

* tslint fix

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@nreese
Copy link
Contributor Author

nreese commented May 5, 2020

I have a suggestion for the text underneath the Format field

@gchaps Would you mind making an issue for these field formatter text changes? That UI is not a part of this UI. The screen shots where just included to show how @thomasneirynck configured some field formatters.

nreese added a commit that referenced this pull request May 5, 2020
…5337)

* [Maps] observability real user monitoring solution layer

* make stateful component

* rename RUM to observability

* layer select

* metrics select

* display select

* clean up

* create cholopleth map layer descriptor

* get styling working

* fix unique count agg

* create geo grid source

* render as heatmap

* clusters

* tslint errors

* last tslint fix

* only show card when APM index exists

* layer label

* clean up

* fix getLayerWizards

* add parans

* review feedback

* add cluster labels

* use green to red ramp

* tslint fix

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:enhancement Team:APM All issues that need APM UI Team support v7.8.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants