Rename *.time metrics to *.time.ms #6449
Conversation
|
I think we should add the new one keep the older and remove them in 7. |
|
Sending the old fields along the new field will probably break old version of x-pack? |
|
@ph @tsullivan and me agreed on Slack that it's ok to rename those fields. In XPack CPU usage metrics appear in 6.3 first. Possible existing users of these metrics were already be warned that field names might change between releases. |
libbeat/cmd/instance/metrics.go
Outdated
| @@ -122,16 +122,16 @@ func reportBeatCPU(_ monitoring.Mode, V monitoring.Visitor) { | |||
|
|
|||
| monitoring.ReportNamespace(V, "user", func() { | |||
| monitoring.ReportInt(V, "ticks", int64(cpuTicks.User)) | |||
| monitoring.ReportInt(V, "time", userTime) | |||
| monitoring.ReportInt(V, "time_ms", userTime) | |||
There was a problem hiding this comment.
Can we go with time.ms to follow our metricbeat convention?
There was a problem hiding this comment.
Would that allow the field name of the monitoring document to keep the time_ms name?
There was a problem hiding this comment.
@tsullivan I assume the metrics in the monitoring document are in sync with what we have here so it's time?
There was a problem hiding this comment.
@ruflin the concern with naming the field time is that it's ambiguous on whether this is a measure of milliseconds or a unix epoch value. That's our x-pack monitoring data mappings don't have any fields named time and we want to stay consistent with that practice.
Whether it is time_ms or time.ms is fine with me. My concern with time.ms is that the dot should represent a path in the json, not a dot in the field name.
|
@kvch Can you modify this PR to make |
kvch
force-pushed
the
feature/libbeat/rename-cpu-time
branch
from
caf6cc1
to
4683def
Compare
|
@ruflin Done. I am not sure if a changelog entry is needed, but I added one. If it's not required here, I will drop the commit. |
| @@ -23,6 +23,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di | |||
| - De dot keys of labels and annotations in kubernetes meta processors to prevent collisions. {pull}6203[6203] | |||
| - The default value for pipelining is reduced to 2 to avoid high memory in the Logstash beats input. {pull}6250[6250] | |||
| - Add logging when monitoring cannot connect to Elasticsearch. {pull}6365[6365] | |||
| - Rename beat.cpu.*.time_ms metrics to beat.cpu.*.time.ms. {pull}6449[6449] | |||
There was a problem hiding this comment.
Is beat.cpu.*.time_ms correct? Should that be beat.cpu.*.time?
Updating the PR title to match the final change would be good as well.
There was a problem hiding this comment.
No, it's not correct. Fun.
ruflin
changed the title
The following renames are done as requested by @tsullivan in https://github.com/elastic/x-pack-kibana/issues/4644#issuecomment-367526912:
beat.cpu.system.time->beat.cpu.system.time.msbeat.cpu.user.time->beat.cpu.user.time.msbeat.cpu.total.time->beat.cpu.total.time.msI am not sure if it's ok to rename these metrics, as they are already released. I am closing if we decide against renaming.
EDIT: the following counters can be access like this: event["beat"]["cpu"]["user"]["time"]["ms"], etc.