Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Metricbeat] Add 'query' metricset for prometheus module #17104

Merged
merged 29 commits into from
Mar 23, 2020
Merged

[Metricbeat] Add 'query' metricset for prometheus module #17104

merged 29 commits into from
Mar 23, 2020

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Mar 19, 2020 •
edited
Loading

What does this PR do?

Adds 'query' metricset for prometheus module. It continues the PR #15177, and aims to the cleanups/changes required.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works

How to test this PR locally

  1. Start a Prometheus container: docker run -p 9090:9090 prom/prometheus
  2. Enable Prometheus module of Metricbeat and test with the following cases:
    a.
- module: prometheus
  period: 10s
  hosts: ["localhost:9090"]
  metricsets: ["query"]
  queries:
  - query_name: "string"
    path: "/api/v1/query"
    query_params:
      query: "some_value"

b.

- module: prometheus
  period: 10s
  hosts: ["localhost:9090"]
  metricsets: ["query"]
  queries:
  - query_name: "scalar"
    path: "/api/v1/query"
    query_params:
      query: "100"

c.

- module: prometheus
  period: 10s
  hosts: ["localhost:9090"]
  metricsets: ["query"]
  queries:
  - query_name: "instant_vector"
    path: "/api/v1/query"
    query_params:
      query: "sum(rate(prometheus_http_requests_total[1m]))"

d.
Change time range accordingly. Also some time is needed so the metrics to have been populated in Prometheus for the respective range

- module: prometheus
  period: 10s
  hosts: ["localhost:9090"]
  metricsets: ["query"]
  queries:
  - query_name: "range_vector"
    path: "/api/v1/query_range"
    query_params:
      query: "up"
      start: "2020-03-20T00:00:00.000Z"
      end:  "2020-03-20T00:00:10.000Z"
      step: 1m

Related issues

estherk0 and others added 10 commits December 21, 2019 18:50
@estherk0
[Metricbeat] Add 'query' metricset for prometheus module
7126e8a
@estherk0
refactor the output structure for Prometheus query API
4d6e4fa 
* Remove array from response body.  Original response body has an array (Vector type). It makes difficult
to query with Elasticsearch QL.
New data schema:
  "prometheus": {
    "query": {
      "mem_usage": {
        "status": "success",
        "data": {
          "resultType": "vector",
          "result": [
            {
              "metric": {},
              "reconciledValue": {
                "unixtimestamp": 1.576751116531e+09,
                "value": "2947656593408"
              }
            }
          ]
        }
      }
    }
  }
@estherk0
[Metricbeat] Support all expression query result formats
c511e95 
* reference url: https://prometheus.io/docs/prometheus/latest/querying/api/#expression-query-result-formats
@estherk0
[Metricbeat] convert numeric string to integer/float type
bc303c6 
* Prometheus API returns "string" type for query result. But actual
result type is a number.
* Make it easy to use ES SQL
@ChrsMark
Merge branch 'support-prom-api-query' of https://github.com/openinfra…
be76be0 
…dev/beats into openinfradev-support-prom-api-query
@ChrsMark
wip
55de062 
Signed-off-by: chrismark <[email protected]>
@ChrsMark
Fix fields, docs, config
5bddf26 
Signed-off-by: chrismark <[email protected]>
@ChrsMark
refactor and cleanups
e27ba4a 
Signed-off-by: chrismark <[email protected]>
@ChrsMark
Add unit&system tests
8d2de92 
Signed-off-by: chrismark <[email protected]>
@ChrsMark
Add system tests
c87af0f 
Signed-off-by: chrismark <[email protected]>
@ChrsMark ChrsMark added enhancement review Metricbeat Metricbeat [zube]: In Review Team:Platforms Label for the Integrations - Platforms team labels Mar 19, 2020
@ChrsMark ChrsMark requested review from exekias and a team March 19, 2020 08:33
@ChrsMark ChrsMark self-assigned this Mar 19, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-platforms (Team:Platforms)

@ChrsMark
Add changelog and fixes
ccc1e54 
Signed-off-by: chrismark <[email protected]>
sayden
sayden reviewed Mar 19, 2020
View reviewed changes
Copy link
Contributor

@sayden sayden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code needs some improvements, but not so much

metricbeat/module/prometheus/query/data.go Outdated Show resolved Hide resolved
metricbeat/module/prometheus/query/data.go Outdated Show resolved Hide resolved
metricbeat/module/prometheus/query/data.go Outdated Show resolved Hide resolved
metricbeat/module/prometheus/query/query.go Outdated Show resolved Hide resolved
metricbeat/module/prometheus/query/query.go Show resolved Hide resolved
metricbeat/module/prometheus/query/query.go Outdated Show resolved Hide resolved
ChrsMark added 2 commits March 19, 2020 11:39
@ChrsMark
make update
9e414dc 
Signed-off-by: chrismark <[email protected]>
@ChrsMark
review fixes
5568fd4 
Signed-off-by: chrismark <[email protected]>
ChrsMark added 2 commits March 19, 2020 20:23
@ChrsMark
fixes
509d62c 
Signed-off-by: chrismark <[email protected]>
@ChrsMark
Increase promtetheus warmup waiting time
b7a50a7 
Signed-off-by: chrismark <[email protected]>
@ChrsMark ChrsMark added the test-plan Add this PR to be manual test plan label Mar 20, 2020
@ChrsMark
Copy link
Member Author

jenkins, test this again

@ChrsMark
Copy link
Member Author

ChrsMark commented Mar 20, 2020 •
edited
Loading

@exekias this seems to be ready for merge, however would you like to have a look too since you had done the initial review on the initial PR?

exekias
exekias reviewed Mar 23, 2020
View reviewed changes
Copy link
Contributor

@exekias exekias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this! Just a few minor questions and comments.

It would be nice to add tests for the other types of queries (range/matrix/string)

metricbeat/module/prometheus/query/config.go Outdated Show resolved Hide resolved
metricbeat/module/prometheus/query/_meta/data.json Show resolved Hide resolved
ChrsMark added 2 commits March 23, 2020 12:05
@ChrsMark
fixes
f8c2501 
Signed-off-by: chrismark <[email protected]>
@ChrsMark
put __name__ labels back
c76cfc9 
Signed-off-by: chrismark <[email protected]>
@ChrsMark ChrsMark merged commit 7c82034 into elastic:master Mar 23, 2020
ChrsMark added a commit to ChrsMark/beats that referenced this pull request Mar 23, 2020
@ChrsMark
[Metricbeat] Add 'query' metricset for prometheus module (elastic#17104)
58b3d1d 
Co-authored-by: esther kim <[email protected]>
(cherry picked from commit 7c82034)
@ChrsMark ChrsMark mentioned this pull request Mar 23, 2020
Merged
5 tasks
ChrsMark added a commit that referenced this pull request Mar 23, 2020
@ChrsMark
[Metricbeat] Add 'query' metricset for prometheus module (#17104) (#1…
952e72d 
…7178)

Co-authored-by: esther kim <[email protected]>
(cherry picked from commit 7c82034)
@andresrc andresrc added test-plan-added This PR has been added to the test plan and removed [zube]: Done labels Mar 27, 2020
@sayden sayden assigned sayden and ChrsMark and unassigned ChrsMark and sayden Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sayden sayden sayden approved these changes

@exekias exekias exekias approved these changes

Assignees

@ChrsMark ChrsMark

Labels
enhancement Metricbeat Metricbeat review Team:Platforms Label for the Integrations - Platforms team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.7.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ChrsMark @elasticmachine @exekias @sayden @andresrc @estherk0