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.

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

Add MySQL query metricset with lightweight module and SQL helper #18955

Merged
merged 24 commits into from
Jul 13, 2020

Conversation

sayden
Copy link
Contributor

@sayden sayden commented Jun 3, 2020

What does this PR do?

This PR adds a SQL helper (at Metricbeat level) with logic to execute SQL queries and return their results. The module can use this abstraction layer to simply pass in the SQL driver and the queries they need to execute to later create Beats events.

This PR introduce:

  • Major changes in sql module.
  • Completely new SQL helper with refactored code from sql module.
  • query Metricset in MySQL module which uses the SQL helper. This Metricset is to use it as a Lightweight "parent" module.
  • performance metricset using the query metricset as Lightweight parent module. Use it as a guidance about how to make it work.
  • overview metricset with a bunch of key metrics, also using Lightweight modules. This is the metricset to remove of this PR. Removed
  • Rework of the SQL module to use the SQL helper too. Basically the code from sql module was moved to the sql helper.

Why is it important?

The way the sql was working was a bit limited compared to the underlying power it has. Moving it one layer up from the modules, allows the use of its logic by any module while maintaining each module functionality regarding specific DSN or security, for example, instead of introducing all that logic into the sql module.

The way it works now also allows to create metricsets on CockroachDB module using lightweight modules.

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
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

Related issues

Fixes: #18898
Implements: #15048

@sayden sayden added enhancement Metricbeat Metricbeat Team:Services (Deprecated) Label for the former Integrations-Services team labels Jun 3, 2020
@sayden sayden self-assigned this Jun 3, 2020
@sayden sayden changed the title Add MySQL query metricset with lightweight module and new query helper Add MySQL query metricset with lightweight module and SQL helper Jun 3, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jun 3, 2020

💔 Tests Failed

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #18955 updated]

  • Start Time: 2020-07-13T12:40:53.872+0000

  • Duration: 77 min 43 sec

Test stats 🧪

Test Results
Failed 1
Passed 2850
Skipped 672
Total 3523

Test errors

Expand to view the tests failures

  • Name: Build and Test / Metricbeat OSS Unit tests / test_process – test_system.Test

    • Age: 1
    • Duration: 1.457
    • Error Details: False is not true : fd not found in any process events

Steps errors

Expand to view the steps failures

  • Name: Mage build unitTest

    • Description: mage build unitTest

    • Duration: 6 min 55 sec

    • Start Time: 2020-07-13T13:04:44.774+0000

    • log

  • Name: Install Go 1.14.4

    • Description: .ci/scripts/install-go.sh

    • Duration: 1 min 57 sec

    • Start Time: 2020-07-13T13:04:31.648+0000

    • log

  • Name: Mage goIntegTest

    • Description: mage goIntegTest

    • Duration: 5 min 39 sec

    • Start Time: 2020-07-13T13:05:41.215+0000

    • log

  • Name: Mage pythonIntegTest

    • Description: mage pythonIntegTest

    • Duration: 3 min 28 sec

    • Start Time: 2020-07-13T13:05:03.839+0000

    • log

  • Name: Mage build test

    • Description: mage build test

    • Duration: 8 min 30 sec

    • Start Time: 2020-07-13T13:07:24.782+0000

    • log

  • Name: Report to Codecov

    • Description: curl -sSLo codecov https://codecov.io/bash for i in auditbeat filebeat heartbeat libbeat metricbeat packetbeat winlogbeat journalbeat do FILE="${i}/build/coverage/full.cov" if [ -f "${FILE}" ]; then bash codecov -f "${FILE}" fi done

    • Duration: 2 min 22 sec

    • Start Time: 2020-07-13T13:15:59.086+0000

    • log

Log output

Expand to view the last 100 lines of log output

[2020-07-13T13:53:51.624Z] + FILE=metricbeat/build/coverage/full.cov
[2020-07-13T13:53:51.624Z] + '[' -f metricbeat/build/coverage/full.cov ']'
[2020-07-13T13:53:51.624Z] + for i in auditbeat filebeat heartbeat libbeat metricbeat packetbeat winlogbeat journalbeat
[2020-07-13T13:53:51.624Z] + FILE=packetbeat/build/coverage/full.cov
[2020-07-13T13:53:51.624Z] + '[' -f packetbeat/build/coverage/full.cov ']'
[2020-07-13T13:53:51.624Z] + for i in auditbeat filebeat heartbeat libbeat metricbeat packetbeat winlogbeat journalbeat
[2020-07-13T13:53:51.624Z] + FILE=winlogbeat/build/coverage/full.cov
[2020-07-13T13:53:51.624Z] + '[' -f winlogbeat/build/coverage/full.cov ']'
[2020-07-13T13:53:51.624Z] + for i in auditbeat filebeat heartbeat libbeat metricbeat packetbeat winlogbeat journalbeat
[2020-07-13T13:53:51.624Z] + FILE=journalbeat/build/coverage/full.cov
[2020-07-13T13:53:51.624Z] + '[' -f journalbeat/build/coverage/full.cov ']'
[2020-07-13T13:53:53.585Z] Post stage
[2020-07-13T13:53:53.594Z] Running in /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats
[2020-07-13T13:53:54.405Z] Starting "default"...
[2020-07-13T13:53:55.340Z] (default) Check network to re-create if needed...
[2020-07-13T13:53:56.830Z] (default) Waiting for an IP...
[2020-07-13T13:54:47.245Z] Machine "default" was started.
[2020-07-13T13:54:47.245Z] Waiting for SSH to be available...
[2020-07-13T13:54:47.245Z] Detecting the provisioner...
[2020-07-13T13:54:47.245Z] Started machines may have new IP addresses. You may need to re-run the `docker-machine env` command.
[2020-07-13T13:54:47.245Z] Client: Docker Engine - Community
[2020-07-13T13:54:47.245Z]  Version:           19.03.1
[2020-07-13T13:54:47.245Z]  API version:       1.40
[2020-07-13T13:54:47.245Z]  Go version:        go1.12.5
[2020-07-13T13:54:47.245Z]  Git commit:        74b1e89
[2020-07-13T13:54:47.245Z]  Built:             Thu Jul 25 21:18:17 2019
[2020-07-13T13:54:47.245Z]  OS/Arch:           darwin/amd64
[2020-07-13T13:54:47.245Z]  Experimental:      false
[2020-07-13T13:54:47.245Z] 
[2020-07-13T13:54:47.245Z] Server: Docker Engine - Community
[2020-07-13T13:54:47.245Z]  Engine:
[2020-07-13T13:54:47.245Z]   Version:          19.03.5
[2020-07-13T13:54:47.245Z]   API version:      1.40 (minimum version 1.12)
[2020-07-13T13:54:47.245Z]   Go version:       go1.12.12
[2020-07-13T13:54:47.245Z]   Git commit:       633a0ea838
[2020-07-13T13:54:47.246Z]   Built:            Wed Nov 13 07:28:45 2019
[2020-07-13T13:54:47.246Z]   OS/Arch:          linux/amd64
[2020-07-13T13:54:47.246Z]   Experimental:     false
[2020-07-13T13:54:47.246Z]  containerd:
[2020-07-13T13:54:47.246Z]   Version:          v1.2.10
[2020-07-13T13:54:47.246Z]   GitCommit:        b34a5c8af56e510852c35414db4c1f4fa6172339
[2020-07-13T13:54:47.246Z]  runc:
[2020-07-13T13:54:47.246Z]   Version:          1.0.0-rc8+dev
[2020-07-13T13:54:47.246Z]   GitCommit:        3e425f80a8c931f88e6d94a8c831b9d5aa481657
[2020-07-13T13:54:47.246Z]  docker-init:
[2020-07-13T13:54:47.246Z]   Version:          0.18.0
[2020-07-13T13:54:47.246Z]   GitCommit:        fec3683
[2020-07-13T13:57:15.200Z] Running in /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats
[2020-07-13T13:57:15.508Z] + find . -type f -name TEST*.xml -path */build/* -delete
[2020-07-13T13:57:15.519Z] Running in /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats/Lint
[2020-07-13T13:57:15.648Z] Running in /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats/Metricbeat-Python-integration-tests
[2020-07-13T13:57:15.731Z] Running in /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats/Metricbeat-OSS-Integration-tests
[2020-07-13T13:57:15.807Z] Running in /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats/Metricbeat-OSS-Unit-tests
[2020-07-13T13:57:15.883Z] Running in /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats/Metricbeat-x-pack
[2020-07-13T13:57:15.964Z] Running in /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats/Metricbeat-crosscompile
[2020-07-13T13:57:16.040Z] Running in /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats/Metricbeat-Windows
[2020-07-13T13:57:16.128Z] Running in /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats/Metricbeat-x-pack-Windows
[2020-07-13T13:57:16.211Z] Running in /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats/Metricbeat-Mac-OS-X
[2020-07-13T13:57:16.289Z] Running in /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats/Metricbeat-x-pack-Mac-OS-X
[2020-07-13T13:57:16.679Z] + cat
[2020-07-13T13:57:16.679Z] + /usr/local/bin/runbld ./runbld-script
[2020-07-13T13:57:16.679Z] Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8
[2020-07-13T13:57:23.265Z] runbld>>> runbld started
[2020-07-13T13:57:23.265Z] runbld>>> 1.6.12/f45d832f2ba0aa2722ab4ec1fda8ad140f027f8b
[2020-07-13T13:57:24.207Z] runbld>>> The following profiles matched the job 'Beats/beats/PR-18955' in order of occurrence in the config (last value wins).
[2020-07-13T13:57:25.588Z] runbld>>> Debug logging enabled.
[2020-07-13T13:57:25.588Z] runbld>>> Storing result
[2020-07-13T13:57:25.848Z] runbld>>> Store result: created {:total 2, :successful 2, :failed 0} 1
[2020-07-13T13:57:25.848Z] runbld>>> BUILD: https://c150076387b5421f9154dfbf536e5c60.us-west1.gcp.cloud.es.io:9243/build-1587637540455/t/20200713135725-A48A26F2
[2020-07-13T13:57:25.848Z] runbld>>> Adding system facts.
[2020-07-13T13:57:26.790Z] runbld>>> Adding vcs info for the latest commit:  d59d98c59f68b99690fd980b86846767afb8c22c
[2020-07-13T13:57:26.790Z] runbld>>> >>>>>>>>>>>> SCRIPT EXECUTION BEGIN >>>>>>>>>>>>
[2020-07-13T13:57:26.790Z] runbld>>> Adding /usr/lib/jvm/java-8-openjdk-amd64/bin to the path.
[2020-07-13T13:57:26.790Z] Processing JUnit reports with runbld...
[2020-07-13T13:57:26.790Z] + echo 'Processing JUnit reports with runbld...'
[2020-07-13T13:57:27.052Z] runbld>>> <<<<<<<<<<<< SCRIPT EXECUTION END <<<<<<<<<<<<
[2020-07-13T13:57:27.052Z] runbld>>> DURATION: 27ms
[2020-07-13T13:57:27.052Z] runbld>>> STDOUT: 40 bytes
[2020-07-13T13:57:27.052Z] runbld>>> STDERR: 49 bytes
[2020-07-13T13:57:27.052Z] runbld>>> WRAPPED PROCESS: SUCCESS (0)
[2020-07-13T13:57:27.052Z] runbld>>> Searching for build metadata in /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats
[2020-07-13T13:57:27.992Z] runbld>>> Storing build metadata: 
[2020-07-13T13:57:27.992Z] runbld>>> Adding test report.
[2020-07-13T13:57:27.992Z] runbld>>> Searching for junit test output files with the pattern: TEST-.*\.xml$ in: /var/lib/jenkins/workspace/Beats_beats_PR-18955/src/github.com/elastic/beats
[2020-07-13T13:57:28.932Z] runbld>>> Found 12 test output files
[2020-07-13T13:57:29.874Z] runbld>>> Test output logs contained: Errors: 0 Failures: 1 Tests: 3523 Skipped: 597
[2020-07-13T13:57:29.874Z] runbld>>> Storing result
[2020-07-13T13:57:29.874Z] runbld>>> FAILURES: 1
[2020-07-13T13:57:30.446Z] runbld>>> Store result: updated {:total 2, :successful 2, :failed 0} 2
[2020-07-13T13:57:30.446Z] runbld>>> BUILD: https://c150076387b5421f9154dfbf536e5c60.us-west1.gcp.cloud.es.io:9243/build-1587637540455/t/20200713135725-A48A26F2
[2020-07-13T13:57:30.446Z] runbld>>> Email notification disabled by environment variable.
[2020-07-13T13:57:30.446Z] runbld>>> Slack notification disabled by environment variable.
[2020-07-13T13:57:36.434Z] Running on Jenkins in /var/lib/jenkins/workspace/Beats_beats_PR-18955
[2020-07-13T13:57:36.653Z] [INFO] getVaultSecret: Getting secrets
[2020-07-13T13:57:36.728Z] Masking supported pattern matches of $VAULT_ADDR or $VAULT_ROLE_ID or $VAULT_SECRET_ID
[2020-07-13T13:57:37.447Z] + chmod 755 generate-build-data.sh
[2020-07-13T13:57:37.448Z] + ./generate-build-data.sh https://beats-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/Beats/beats/PR-18955/ https://beats-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/Beats/beats/PR-18955/runs/2 FAILURE 4603315
[2020-07-13T13:57:37.448Z] INFO: curl https://beats-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/Beats/beats/PR-18955/runs/2/steps/?limit=10000 -o steps-info.json
[2020-07-13T13:57:37.998Z] INFO: curl https://beats-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/Beats/beats/PR-18955/runs/2/tests/?status=FAILED -o tests-errors.json
[2020-07-13T13:57:38.248Z] INFO: curl https://beats-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/Beats/beats/PR-18955/runs/2/log/ -o pipeline-log.txt

@sayden
Copy link
Contributor Author

sayden commented Jul 6, 2020

I'm thinking to remove the overview metricset to make this PR smaller and easier to review while we re-think what we want to include in the overview metricset or if we want to include it at all.

Having it as a lightweight module, it is easy to include it later.

@sayden sayden marked this pull request as ready for review July 6, 2020 14:32
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-services (Team:Services)

@jsoriano jsoriano self-requested a review July 6, 2020 16:03
Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

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

Nice feature! I left some comments but in general looks good!

metricbeat/docs/modules/sql.asciidoc Outdated Show resolved Hide resolved
metricbeat/docs/modules/sql.asciidoc Outdated Show resolved Hide resolved
metricbeat/docs/modules/sql.asciidoc Show resolved Hide resolved
metricbeat/helper/sql/sql.go Show resolved Hide resolved
metricbeat/helper/sql/sql.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/sql/query/query.go Outdated Show resolved Hide resolved
metricbeat/module/mysql/overview/overview.go Outdated Show resolved Hide resolved
metricbeat/module/mysql/overview/manifest.yml Outdated Show resolved Hide resolved
metricbeat/helper/sql/sql_test.go Outdated Show resolved Hide resolved
metricbeat/helper/sql/sql.go Outdated Show resolved Hide resolved
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

This is looking good, I think that is a good idea to have generic metricsets on the specific modules to have documentation together and a more consistent experience for users using an only db.

Changelog entries will be needed, there are many things here we should mention:

  • New query metricset in the mysql module.
  • New performance metricset in the mysql module.
  • Add support for booleans in the generic sql module.
  • Null values are not reported in the generic sql module.

Maybe for the developer changelog:

  • New sql helper for metricbeat modules.

metricbeat/module/mysql/performance/_meta/fields.yml Outdated Show resolved Hide resolved
metricbeat/module/mysql/performance/_meta/fields.yml Outdated Show resolved Hide resolved
metricbeat/module/mysql/query/_meta/fields.yml Outdated Show resolved Hide resolved
metricbeat/module/mysql/query/query.go Outdated Show resolved Hide resolved
metricbeat/module/mysql/query/query.go Outdated Show resolved Hide resolved
metricbeat/helper/sql/sql.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/sql/_meta/docs.asciidoc Outdated Show resolved Hide resolved
metricbeat/helper/sql/sql.go Outdated Show resolved Hide resolved
metricbeat/module/mysql/query/query.go Show resolved Hide resolved
x-pack/metricbeat/module/sql/query/query.go Show resolved Hide resolved
@sayden
Copy link
Contributor Author

sayden commented Jul 7, 2020

Very good comments indeed. Thanks! Will take a look

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Please try to regenerate the data.json files after adding the host parser and the replace_underscores option to the manifest.

metricbeat/module/mysql/performance/_meta/data.json Outdated Show resolved Hide resolved
metricbeat/module/mysql/performance/manifest.yml Outdated Show resolved Hide resolved
metricbeat/module/mysql/query/query.go Show resolved Hide resolved
x-pack/metricbeat/module/sql/query/query.go Show resolved Hide resolved
Copy link
Contributor

@kvch kvch left a comment

Choose a reason for hiding this comment

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

LGTM

driver: "postgres"
sql_query: "SELECT * FROM pg_catalog.pg_tables pt WHERE schemaname ='pg_catalog'"
sql_response_format: table
---
Copy link
Member

Choose a reason for hiding this comment

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

This should fix the doc build

Suggested change
---
----

Copy link
Member

Choose a reason for hiding this comment

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

Oh but this is generated. I don't know where this is is coming from. @dedemorton probably will

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh please, I cannot believe it 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, pushed and fixed. Thank you very much 😃

@sayden sayden merged commit d0f7058 into elastic:master Jul 13, 2020
sayden added a commit to sayden/beats that referenced this pull request Jul 13, 2020
@sayden sayden added the v7.9.0 label Jul 13, 2020
sayden added a commit that referenced this pull request Jul 13, 2020
v1v added a commit to v1v/beats that referenced this pull request Jul 14, 2020
* upstream/master: (25 commits)
  [Elastic Agent] Send checkin payload to Fleet (elastic#19857)
  [Ingest Manager] Fixed tests across agent elastic#19877
  [Ingest Manager] Fix serialization test  elastic#19876
  Fix service start type mapping in windows/service metricset (elastic#19551)
  ci: Change comment trigger detection method (elastic#19827)
  Add 21 autogenerated filesets from rsa2elk devices (elastic#19713)
  [Ingest Manager] Agent config cleanup (elastic#19848)
  libbeat/publisher/pipeline: fix data races (elastic#19821)
  Update monitoring-internal-collection.asciidoc (elastic#19422) (elastic#19697)
  [Elastic Agent] Trust exchange endpoint must bind to 127.0.0.1 (elastic#19861)
  Specify an ECS version in Auditbeat/Packetbeat/Winlogbeat (elastic#19159)
  Add azure billing metricset (elastic#19207)
  Add support for appinsights in the metricbeat azure module (elastic#18940)
  Add MySQL query metricset with lightweight module and SQL helper (elastic#18955)
  [Ingest Manager] Refuse invalid stream values in configuration (elastic#19587)
  Do not use vendor during integration tests (elastic#19839)
  LIBBEAT: Enhancement Convert dissected values from String to other basic data types and IP (elastic#18683)
  [Elastic Agent] Remove support for "logs" and only support logfile (elastic#19761)
  [CI] support windows-2012 (elastic#19773)
  Do not update go.mod during packaging and testing (elastic#19823)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Metricbeat Metricbeat Team:Services (Deprecated) Label for the former Integrations-Services team v7.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Metricbeat] SQL module converts NULL cells to strings
8 participants