Add kafka fields needed for dashboards #8504
Conversation
Kafka dashboards calculate cardinalities of composed fields, this is expensive in query time, but can be optimized if the composed fields are precalculated. Continues with elastic#7767, needed for elastic#8457.
jsoriano
force-pushed
the
kafka-id-fields
branch
from
3bee75a to
5a0a8b8
Compare
jsoriano
added
review
needs_backport
CHANGELOG.asciidoc
Outdated
| @@ -141,6 +141,7 @@ https://github.com/elastic/beats/compare/v6.4.0...master[Check the HEAD diff] | |||
| - Support for Kafka 2.0.0 {pull}8399[8399] | |||
| - Add container image for docker metricsets. {issue}8214[8214] {pull}8438[8438] | |||
| - Add support for `full` status page output for php-fpm module as a separate metricset called `process`. {pull}8394[8394] | |||
| - Precalculate composed id fields for kafka dashboards {pull}8504[8504] | |||
There was a problem hiding this comment.
Nit: Dot missing at the end.
| @@ -111,6 +112,10 @@ func (m *MetricSet) Fetch(r mb.ReporterV2) { | |||
| } | |||
|
|
|||
| emitEvent := func(event common.MapStr) { | |||
| // Helpful IDs for dashboards | |||
There was a problem hiding this comment.
I would rather describe it as helpful for queries.
For the future: I wonder if this is work that should be done in the processors to have it config based instead of in the code.
| } | ||
| "name": "foo-1538389014-739473801" | ||
| }, | ||
| "topic_broker_id": "0-foo-1538389014-739473801-0", |
There was a problem hiding this comment.
Why not topic.id and topic.broker_id?
There was a problem hiding this comment.
I am not really sure about the naming and placing of these composed ids, in demo they are directly at the module level as kafka.topic_partition_broker_id, I moved them to partition as in some way they identify the partition. For the same reason I'd keep them as fields directly under the partition, and I wouldn't expect other fields under kafka.partition.topic. kafka.partition.topic.id may be seen also as if it should be the same as kafka.topic.id.
In any case I don't have a strong opinion on these names, so as you prefer.
There was a problem hiding this comment.
I see now that topic.id shows up in different events. So kafka.topic.id would probably make most sense? Meaning that we should move out topic from partition to also have kafka.topic.name (would be a breaking change)?
There was a problem hiding this comment.
kafka.topic.name was added in #7767 to replace kafka.partition.topic.name and kafka.consumergroup.topic.name as a common name. These fields contain the topic name as seen by kafka, old ones are marked as deprecated.
The fields added in this PR are created by us to help on queries and don't exist in kafka, they identify the partition in the topic and in the broker (there can be two partitions with the same id, but of different topics, and two partitions with the same id and different topics and/or brokers), this is why I'd add them under kafka.partition, I see them more like identifiers of the partition.
There was a problem hiding this comment.
Is it possible that that more then one topic.id shows up in a single event?
There was a problem hiding this comment.
Not sure of following you... a single event will have an only kafka.topic.id and an only kafka.partition.topic_id (or however we call it), and both ids will be different, being the second one composed by the first one and kafka.partition.id.
There was a problem hiding this comment.
That actually answers my questions. There are 2 topic.id in 1 event so there have to be different fields like you suggested.
|
@jsoriano Please move forward with your suggestion. Can you rebase this on master? |
|
jenkins, test this again please |
|
I have done some last time changes. The id with the broker ID was using the leader ID, so it generated the same for all replicas (and the same for all events with the same topic and partition). Now I use the replica ID, so it is different and we can calculate things like number of replicas using this. |
jsoriano
removed
the
needs_backport
Kafka dashboards calculate cardinalities of composed fields, this is expensive in query time, but can be optimized if the composed fields are precalculated. Continues with elastic#7767, needed for elastic#8457. (cherry picked from commit 78ef120)
Kafka dashboards calculate cardinalities of composed
fields, this is expensive in query time, but can be optimized if the
composed fields are precalculated.
Continues with #7767, needed for #8457.