fix kibana metricsets property paths #122034
Conversation
klacabane
added
auto-backport
|
Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI) |
klacabane
added
the
release_note:skip
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @klacabane |
| @@ -23,6 +23,7 @@ export default function ({ getService, getPageObjects }) { | |||
| { | |||
| from: 'Aug 29, 2017 @ 17:24:14.254', | |||
| to: 'Aug 29, 2017 @ 17:25:44.142', | |||
| useCreate: true, | |||
There was a problem hiding this comment.
Out of scope: This is set since we're writing to a data stream right? I wonder if perhaps it would be nicer if our setup methods used that domain language instead of "useCreate" which is a detail of how es_archiver decides to put data into ES if it's a data stream?
It's very unclear to me from just reading this call site what this parameter does.
There was a problem hiding this comment.
Agreed, I'll update that as part of the logstash PR
| const legacyStats = hit._source.kibana_stats; | ||
| const mbStats = hit._source.kibana?.stats; | ||
|
|
||
| const kibana: Kibana = { | ||
| kibana: hit._source.kibana?.kibana ?? legacyStats?.kibana, | ||
| kibana: buildKibanaInfo(hit), | ||
| concurrent_connections: | ||
| mbStats?.concurrent_connections ?? legacyStats?.concurrent_connections, | ||
| process: { |
There was a problem hiding this comment.
Maybe out of scope: But for the code below, I wonder if we can write some helper function like getProperty(propertyName, hit), which would hide the legacyDats, mbStats, and all the optional chaining and ?? operators?
I wonder if that would make it a bit more readable?
Internally I'd imagine we give a definite name to each property we use, and two functions that know how to find that property from a hit, then the function itself just grabs these to selectors, and chains them together (i the first one is null, use the second one) to find the values.
There was a problem hiding this comment.
That would be a good improvement for sure. similar conditionals/fallbacks are spread everywhere in the codebase so I'd rather handle this as part of a separate refactoring effort :)
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
* fix kibana metricsets property paths * return inner kibana object * fix functional test * rename * adapt tests to datastream * rebuild archived data * extract buildKibanaInfo for reuse * add license * lint Co-authored-by: Kibana Machine <[email protected]>
* fix kibana metricsets property paths * return inner kibana object * fix functional test * rename * adapt tests to datastream * rebuild archived data * extract buildKibanaInfo for reuse * add license * lint Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kevin Lacabane <[email protected]>
Summary
The metricbeat path for kibana queries currently expects an inner
kibana.stats.kibanaobject to contain a summary of the instance similar to the legacy schema, but this object does not exist. Instead the properties making up this summary are spread in thekibana.statsobject. This change updates codepaths relying on this non-existing object.Also updated the
_mbkibana archives (with this script) used by the apis and functional tests to 1. align with the actual metricbeat schema 2. insert documents in the new datastreams.Testing