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 missing metricsets #8048

Merged
merged 5 commits into from
Sep 18, 2024
Merged

Conversation

kvalliyurnatt
Copy link
Contributor

This PR adds some of the missing metricsets to the stack monitoring configuration
fixes: #7277

@kvalliyurnatt kvalliyurnatt added >enhancement Enhancement of existing functionality v2.15.0 (next) labels Sep 11, 2024
Copy link
Contributor

@naemono naemono left a comment

Choose a reason for hiding this comment

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

I think it's good, but we may want to note if there's a version requirement, see: #7277 (comment)

Also, run the full e2e test suite on this to make sure all versions are ok with this change.... I suspect older versions may fail.

@naemono
Copy link
Contributor

naemono commented Sep 11, 2024

buildkite test this -f p=gke -m s=7.17.8,s=8.15.0,s=8.16.0-SNAPSHOT

@naemono
Copy link
Contributor

naemono commented Sep 11, 2024

Also, run the full e2e test suite on this to make sure all versions are ok with this change.... I suspect older versions may fail.

Update: I kicked them off for you....

@pebrc
Copy link
Collaborator

pebrc commented Sep 12, 2024

Yes I think we need a version gate for this.

@pebrc
Copy link
Collaborator

pebrc commented Sep 13, 2024

Just to capture a discussion we had out of band: the change required here needs to amend the Metricbeat template for the ECK stack monitoring feature. We can also add it to the recipe as done here but the template is the main part and we can use conditionals etc there to make it version dependent

metricsets:
- ccr
- cluster_stats
- enrich
- index
- index_recovery
- index_summary
- ml_job
- node_stats
- pending_tasks
- shard

@kvalliyurnatt
Copy link
Contributor Author

Added a version gate for the ingest pipeline and also added a unit test for it

@kvalliyurnatt
Copy link
Contributor Author

buildkite test this -f p=gke -m s=7.17.8,s=8.15.0,s=8.16.0-SNAPSHOT

@kvalliyurnatt
Copy link
Contributor Author

kvalliyurnatt commented Sep 17, 2024

I tested this locally on my dev cluster
Deployed an Elasticsearch version of 8.3.3

metricbeat:
  modules:
    - hosts:
        - https://localhost:9200/
      metricsets:
        - ccr
        - cluster_stats
        - enrich
        - index
        - index_recovery
        - index_summary
        - ml_job
        - node
        - node_stats
        - pending_tasks
        - shard
        - pending_tasks
      module: elasticsearch
      password: ""
      period: 10s
      ssl:
        certificate_authorities:
          - /mnt/elastic-internal/kb-monitoring/default/source/certs/ca.crt
        enabled: true
        verification_mode: certificate
      username: elastic-internal-monitoring
      xpack:
        enabled: true
output:
  elasticsearch:
    hosts:
      - https://destination-es-http.default.svc:9200/
    password: ""
    ssl:
      certificate_authorities:
        - /mnt/elastic-internal/es-monitoring-association/default/destination/certs/ca.crt
      verification_mode: certificate
    username: default-source-default-destination-beat-es-mon-user
processors:
  - add_cloud_metadata: null
  - add_host_metadata: null

no ingest pipeline in the metric set

deployed Elasticsearch 8.7.0, ingest pipeline present in the metricset

metricbeat:
  modules:
    - hosts:
        - https://localhost:9200/
      metricsets:
        - ccr
        - cluster_stats
        - enrich
        - index
        - index_recovery
        - index_summary
        - ml_job
        - node
        - node_stats
        - pending_tasks
        - shard
        - ingest_pipeline
        - pending_tasks
      module: elasticsearch
      password: ""
      period: 10s
      ssl:
        certificate_authorities:
          - /mnt/elastic-internal/kb-monitoring/default/source/certs/ca.crt
        enabled: true
        verification_mode: certificate
      username: elastic-internal-monitoring
      xpack:
        enabled: true
output:
  elasticsearch:
    hosts:
      - https://destination-es-http.default.svc:9200/
    password: ""
    ssl:
      certificate_authorities:
        - /mnt/elastic-internal/es-monitoring-association/default/destination/certs/ca.crt
      verification_mode: certificate
    username: default-source-default-destination-beat-es-mon-user
processors:
  - add_cloud_metadata: null
  - add_host_metadata: null

Copy link
Contributor

@naemono naemono left a comment

Choose a reason for hiding this comment

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

looks good to me.

config/recipes/beats/stack_monitoring.yaml Outdated Show resolved Hide resolved
@@ -9,9 +9,14 @@ metricbeat.modules:
- index_recovery
- index_summary
- ml_job
- node
Copy link
Contributor

@thbkrkr thbkrkr Sep 18, 2024

Choose a reason for hiding this comment

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

Oh! (I'm so surprised I missed this initially.)

pkg/controller/elasticsearch/stackmon/metricbeat.tpl.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@thbkrkr thbkrkr left a comment

Choose a reason for hiding this comment

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

LGTM

kvalliyurnatt and others added 2 commits September 18, 2024 13:03
Co-authored-by: Thibault Richard <thbkrkr@users.noreply.github.com>
@kvalliyurnatt kvalliyurnatt enabled auto-merge (squash) September 18, 2024 17:23
@kvalliyurnatt kvalliyurnatt merged commit a677aa7 into elastic:main Sep 18, 2024
5 checks passed
thbkrkr added a commit that referenced this pull request Sep 20, 2024
* add missing metricsets, enable ingest pipeline metricset only if version is 8.7.0 and higher

* Update config/recipes/beats/stack_monitoring.yaml

---------

Co-authored-by: Thibault Richard <thbkrkr@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement Enhancement of existing functionality v2.15.0 (next)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include all the metricsets to Metricbeat collected monitoring data
4 participants