[metricbeat] [system] add NTP metricset#44884
[metricbeat] [system] add NTP metricset#44884tommyers-elastic merged 20 commits intoelastic:mainfrom
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
mykola-elastic
left a comment
There was a problem hiding this comment.
Even though it is in "draft" and the checks are not yet fixed, it looks quite good to me
|
This pull request is now in conflicts. Could you fix it? 🙏 |
|
Pinging @elastic/integrations (Team:Integrations) |
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
AndersonQ
left a comment
There was a problem hiding this comment.
I'm trying to test it but it does not work for me.
As you can see in my comments, it seems to me you're not setting the host correctly.
this is how I tested:
- build metricbeat:
go build . - run it
go run . --strict.perms=false -e -c ./metricbeat-anderson.yml - my confg:
path.home: /tmp/beats/home
metricbeat.modules:
- module: system
period: 1s
metricsets:
- ntp
ntp.hosts: ["localhost", "pool.ntp.org"]
output.file:
path: /tmp/beats/home
filename: "output-metricbeat"
logging.level: debug
logging.metrics:
level: debugWhat I'm gerring:
{"log.level":"error","@timestamp":"2025-07-11T16:17:30.550+0200","log.logger":"system.ntp","log.origin":{"function":"github.com/elastic/beats/v7/metricbeat/mb/module.(*metricSetWrapper).handleFetchError","file.name":"module/wrapper.go","file.line":335},"message":"Error fetching data for metricset system.ntp: error querying NTP server : address string is empty","service.name":"metricbeat","ecs.version":"1.6.0"}
{"log.level":"error","@timestamp":"2025-07-11T16:17:31.551+0200","log.logger":"system.ntp","log.origin":{"function":"github.com/elastic/beats/v7/metricbeat/mb/module.(*metricSetWrapper).handleFetchError","file.name":"module/wrapper.go","file.line":335},"message":"Error fetching data for metricset system.ntp: error querying NTP server : address string is empty","service.name":"metricbeat","ecs.version":"1.6.0"}
{"log.level":"error","@timestamp":"2025-07-11T16:17:32.551+0200","log.logger":"system.ntp","log.origin":{"function":"github.com/elastic/beats/v7/metricbeat/mb/module.(*metricSetWrapper).handleFetchError","file.name":"module/wrapper.go","file.line":335},"message":"Error fetching data for metricset system.ntp: error querying NTP server : address string is empty","service.name":"metricbeat","ecs.version":"1.6.0"}
{"log.level":"error","@timestamp":"2025-07-11T16:17:33.551+0200","log.logger":"system.ntp","log.origin":{"function":"github.com/elastic/beats/v7/metricbeat/mb/module.(*metricSetWrapper).handleFetchError","file.name":"module/wrapper.go","file.line":335},"message":"Error fetching data for metricset system.ntp: error querying NTP server : address string is empty","service.name":"metricbeat","ecs.version":"1.6.0"}
{"log.level":"error","@timestamp":"2025-07-11T16:17:34.550+0200","log.logger":"system.ntp","log.origin":{"function":"github.com/elastic/beats/v7/metricbeat/mb/module.(*metricSetWrapper).handleFetchError","file.name":"module/wrapper.go","file.line":335},"message":"Error fetching data for metricset system.ntp: error querying NTP server : address string is empty","service.name":"metricbeat","ecs.version":"1.6.0"}
{"log.level":"error","@timestamp":"2025-07-11T16:17:35.551+0200","log.logger":"system.ntp","log.origin":{"function":"github.com/elastic/beats/v7/metricbeat/mb/module.(*metricSetWrapper).handleFetchError","file.name":"module/wrapper.go","file.line":335},"message":"Error fetching data for metricset system.ntp: error querying NTP server : address string is empty","service.name":"metricbeat","ecs.version":"1.6.0"}
Could you please add a the section "How to test this PR locally" to facilitate others testing it?
|
This pull request is now in conflicts. Could you fix it? 🙏 |
|
@AndersonQ the hosts config should not be prefixed i'll update this - because i see now that if you configured multiple metricsets here it errors out (see output below if you also configure cpu). the NTP config needs to be namespaced. |
ok, let me know when it's ready for re-review |
|
@AndersonQ we should be good to go |
AndersonQ
left a comment
There was a problem hiding this comment.
looks good now. If all tests pass, it's good
colleenmcginnis
left a comment
There was a problem hiding this comment.
Some small suggestions (and big, but not blocking, thoughts) below.
Co-authored-by: Colleen McGinnis <colleen.j.mcginnis@gmail.com>
Co-authored-by: Colleen McGinnis <colleen.j.mcginnis@gmail.com>
Co-authored-by: Colleen McGinnis <colleen.j.mcginnis@gmail.com>
colleenmcginnis
left a comment
There was a problem hiding this comment.
Definition lists look good now!
lalit-satapathy
left a comment
There was a problem hiding this comment.
Code owner approvals.
Proposed commit message
Add new Network Time Protocol (NTP) metricset to the system integration.
This is a simple wrapper around an SNTP client which allows monitoring of system clock offset compared to the configured NTP server.
Example test configuration:
Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.