Add geoip database metrics to /node/stats API#13004
Add geoip database metrics to /node/stats API#13004kaisecheng merged 7 commits intoelastic:masterfrom
Conversation
c6f8cbc to
d885e1b
Compare
andsel
left a comment
There was a problem hiding this comment.
LGTM
Left just a couple of notes on possible improvements for readability
| pipeline_id = ThreadContext.get("pipeline.id") | ||
| ThreadContext.put("pipeline.id", nil) | ||
|
|
||
| @metric.namespace([:download]).gauge(:status, :checking) |
There was a problem hiding this comment.
I think this assignment could be extracted in a method update_download_status(:checking). In this way the intention is more evident without the read immediately deep into the knowledge of metrics.
| ensure | ||
| check_age | ||
| clean_up_database | ||
| set_download_metric(success_cnt) |
There was a problem hiding this comment.
Here we could also use update_download_status so in the flow of download a DB becomes more evident the status change points.
| def set_download_metric(success_cnt) | ||
| @metric.namespace([:download]).tap do |n| | ||
| n.gauge(:last_check_at, Time.now.iso8601) | ||
|
|
||
| if success_cnt == DB_TYPES.size | ||
| n.increment(:successes, 1) | ||
| n.gauge(:status, :succeeded) | ||
| else | ||
| n.increment(:failures, 1) | ||
| n.gauge(:status, :failed) | ||
| end | ||
| end |
There was a problem hiding this comment.
Maybe this could be split in two methods, one for the counter and the other for the status, so that the status update points becomes evident in the download flow
|
After discussion, we decided to improve the wording to reflect the following: geoip -> geoip_download_manager (7.14)
download -> download_stats ? (7.14)
|
This PR adds geoip database status, last update timestamp, download stats counter to Node Stats API
* master: (41 commits) Test: resolve integration failure due ECS mode (elastic#13044) Feat: event factory support (elastic#13017) Doc: Add geoip database API to node stats (elastic#13019) Add geoip database metrics to /node/stats API (elastic#13004) ecs: on-by-default plus docs (elastic#12830) ispec: fix cross-spec leak from fatal error integration specs (elastic#13002) Fix UBI source URL (elastic#13008) update fpm to allow pkg creation on jdk11+jruby 9.2 (elastic#13005) Add unit test to grant that production aliases correspond to a published RubyGem (elastic#12993) Fix logstash.bat not setting exit code (elastic#12948) Use the OS separator to invoke gradlew from Rake script (elastic#13000) Allow per-pipeline config of ECS Compatibility mode via Central Management (elastic#12861) Update jinja2 dependency in docker build (elastic#12994) fix database manager with multiple pipelines (elastic#12862) Fix Reflections stack traces when process yml files in classpath and debug is enabled (elastic#12991) Fix/log4j routing to avoid create spurious file (elastic#12965) Deps: update JRuby to 9.2.19.0 (elastic#12989) Doc: Add tip for checking for existing field (elastic#12899) Added test to cover the installation of aliased plugins (elastic#12967) CI: Update logstash_release.json after 7.3.12 (elastic#12986) ...
|
Docs added: #13019 |
Release notes
Add geoip database metrics to /node/stats
What does this PR do?
This PR exposes geoip database metrics to Logstash localhost:9600/node/stats API
Sample output
Schema
Why is it important/What is the impact to the user?
It allows system administrators to monitor database status through metric API instead of scanning the log.
When they see database status
to_be_expired, they should allow Logstash to access the internet to download the latest database to prevent theexpiredstatus that fails the pluginChecklist
Author's Checklist
How to test this PR locally
please check the
geoipsession ofcurl -XGET 'localhost:9600/_node/stats'Related issues
Use cases
Screenshots
Logs