Adding HTTP metrics collection to Kibana integration - #6169
Conversation
🌐 Coverage report
|
| ## Compatibility | ||
|
|
||
| The `kibana` package works with Kibana 8.5.0 and later. | ||
| The `kibana` package works with Kibana 8.8.0 and later. |
There was a problem hiding this comment.
I assume this change is because of the new dataset? All the other dataset still work for the older versions?
There was a problem hiding this comment.
If the compatibility version in the manifest has changed then you cannot install this (and later) versions of the package on that cluster.
There was a problem hiding this comment.
Yes, this change is because the background task HTTP API endpoint is only available in Kibana 8.8.0+ but all other datasets should still work for older versions.
There was a problem hiding this comment.
Actually realized that the Kibana PR was merged for 8.9 not 8.8 so updated again for 8.9
| "version": "20.04.6 LTS (Focal Fossa)" | ||
| } | ||
| }, | ||
| "http": { |
There was a problem hiding this comment.
I suggest to put this under kibana.background_task_utilization instead.
@jsoriano @joshdover As we have dataset now, I wonder if we still need this prefixing of metrics? Where should we be heading here?
There was a problem hiding this comment.
ECS recommends actually using a prefix to avoid conflicts with future additions to the common schema: https://www.elastic.co/guide/en/ecs/current/ecs-custom-fields-in-ecs.html#_proper_names
Since this is a fairly specific metric that I don't see getting added to ECS, I think a prefix might make sense here.
There was a problem hiding this comment.
I'm not sure I understand what the recommendation is @joshdover, but to chip in, for the other data streams we put them under {product}.{data_stream}.{metric} so I'd prefer to stick to that unless that would cause problems.
There was a problem hiding this comment.
I think we all made the same recommendation only with different words?
There was a problem hiding this comment.
I agree that kibana.background_task_utilization makes sense! Looking at the metricbeat HTTP docs, I only see an option for specifying namespace which seems to automatically add it under the http field (http.${namespace}. Would I need to specify an ingest pipeline to rename it?
There was a problem hiding this comment.
You can either do it with a rename in the agent or in the ingest pipline. Ideally, you could just specific the prefix :-( Worth adding a beats issue to get this fixed long term?
There was a problem hiding this comment.
I added the rename in the agent and opened an issue in beats: elastic/beats#35443
| show_user: true | ||
| default: | ||
| - http://localhost:5601 | ||
| - name: username |
There was a problem hiding this comment.
Should we recommend usage of API keys?
There was a problem hiding this comment.
Maybe...
But, the Metricbeat module that powers the other data streams here do not support using API keys (see elastic/beats#29271), which might lead to some confusion?
Ideally we would prioritize adding API key support for the Metricbeat modules and then add it into each Integration as well.
|
@miltonhultgren Would be great to get your review on this one. I'm also curious to hear your thoughts on the grouping. The |
|
I'll have a look but I wanted to clarify (as usual) that this is meant only for Agent based collection? @ruflin The grouping makes sense in the way that it's separate from the things that power the Stack Monitoring UI. You could add more data streams to the same type though I'm not sure if the label for that group should be the name of the "module" or "technical interface" (the Stack Monitoring stuff also hits the HTTP API for metrics). In other places I think we just call it "Metrics". The main point is that we're free to name this new group anything we want. |
miltonhultgren
left a comment
There was a problem hiding this comment.
Overall this LGTM!
I had some questions/concerns about the data stream naming and the field prefixes and some nitpicks on the copytext.
| # newer versions go on top | ||
| - version: "2.3.5" | ||
| changes: | ||
| - description: Add HTTP metrics to collect background task utilization metric |
There was a problem hiding this comment.
NIT:
| - description: Add HTTP metrics to collect background task utilization metric | |
| - description: Add background task utilization metric |
|
|
||
| ### Background task utilization | ||
|
|
||
| Background task utilization data stream uses the `/api/task_manager/_background_task_utilization` API of Kibana, which is available starting in 8.8. |
There was a problem hiding this comment.
NIT:
| Background task utilization data stream uses the `/api/task_manager/_background_task_utilization` API of Kibana, which is available starting in 8.8. | |
| The Background task utilization data stream uses the `/api/task_manager/_background_task_utilization` API of Kibana, which is available starting in 8.8. |
or
| Background task utilization data stream uses the `/api/task_manager/_background_task_utilization` API of Kibana, which is available starting in 8.8. | |
| This data stream uses the `/api/task_manager/_background_task_utilization` API of Kibana, which is available starting in 8.8. |
| | timestamp | | alias | | ||
|
|
||
|
|
||
| An example event for `background_task_utilization` looks as following: |
There was a problem hiding this comment.
NIT:
| An example event for `background_task_utilization` looks as following: | |
| An example event for `background_task_utilization` looks as follows: |
There was a problem hiding this comment.
This document is auto-generated using the event placeholder {{event "background_task_utilization"}} in packages/kibana/_dev/build/docs/README.md so I can't change this wording without changing the auto-generation, which will likely change all the docs files in this repo 🙈
| type: metrics | ||
| title: Kibana background task utilization metrics | ||
| release: beta | ||
| dataset: kibana.http_metrics.background_task_utilization |
There was a problem hiding this comment.
| dataset: kibana.http_metrics.background_task_utilization | |
| dataset: kibana.background_task_utilization |
I don't think it's needed to add the type into the dataset name. We've added that to the Stack Monitoring legacy streams to not block the usage of that name for when/if new data streams are introduced for the "same" data but natively to the Integration.
| title: Kibana background task utilization metrics | ||
| description: Collect Kibana background task utilization metrics |
There was a problem hiding this comment.
It would be nice if we could include some information about what those metrics are and what they are useful for.
NIT:
Maybe we don't need to write Kibana everywhere since the context should be clear to the user from being on the Kibana integration page?
| "version": "20.04.6 LTS (Focal Fossa)" | ||
| } | ||
| }, | ||
| "http": { |
There was a problem hiding this comment.
I'm not sure I understand what the recommendation is @joshdover, but to chip in, for the other data streams we put them under {product}.{data_stream}.{metric} so I'd prefer to stick to that unless that would cause problems.
@miltonhultgren Yes, this is only meant for agent-based collection. We did not update the Kibana module in metricbeat. |
|
@miltonhultgren I had to update the fields mappings for some of the other datastreams for 8.9 to get the tests to pass: e8d1489 Is this the right way to do that? |
|
@ymao1 Yes, the Integration system tests asserts that all fields in the ingested document have a mapping (which is good/bad) so that's the right way forward 👍🏼 |
|
Package kibana - 2.3.5 containing this change is available at https://epr.elastic.co/search?package=kibana |
* Initial commit * Cleanup * Fixing checks * Updating README: * Updating docs * Updating kibana version to 8.8 * PR feedback * Updating fields for 8.9 * Updating kibana version to 8.9
What does this PR do?
Updates the Kibana integration to use the metricbeat HTTP module to collect background task utilization metrics from the Kibana API endpoint added in this PR.
Checklist
changelog.ymlfile.Author's Checklist
How to test this PR locally
elastic-package:elastic-package stack up --version=8.9.0-SNAPSHOT -v -dcd integrations/packages/kibanaelastic-package buildelastic-package stack up -v -d --services package-registryHTTP metrics background task utilizationintegrationhttps://kibana:5601and username/passwordelastic:changememetrics-kibana.background_task_utilization-defaultGET metrics-kibana.background_task_utilization-default/_search) with thekibana.background_task_utilization.stats.value.loadfield populatedRelated issues
Screenshots