Skip to content

[Elasticsearch][Ingest Pipeline] Add elasticsearch.node.name as TSDS dimension - #11527

Merged
consulthys merged 3 commits into
elastic:mainfrom
BenB196:elasticsearch-ingest-pipeline-dimension-fix
Nov 1, 2024
Merged

[Elasticsearch][Ingest Pipeline] Add elasticsearch.node.name as TSDS dimension#11527
consulthys merged 3 commits into
elastic:mainfrom
BenB196:elasticsearch-ingest-pipeline-dimension-fix

Conversation

@BenB196

@BenB196 BenB196 commented Oct 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Bug

Proposed commit message

Adds elasticsearch.node.name as a TSDS dimension to prevent document collisions.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.
  • I have verified that any added dashboard complies with Kibana's Dashboard good practices

Author's Checklist

  • [ ]

How to test this PR locally

Install assets for the Elasticsearch integration and validate that the metrics-elasticsearch.ingest_pipeline@package component template properly sets elasticsearch.node.name as "time_series_dimension": true

Related issues

Screenshots

image
@BenB196
BenB196 requested a review from a team as a code owner October 26, 2024 12:37
consulthys
consulthys previously approved these changes Oct 26, 2024
View reviewed changes

@consulthys consulthys left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well spotted!
Thanks for the contribution
@consulthys

consulthys commented Oct 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Interestingly for the node_stats metric set, the elasticsearch.node.name was added as a dimension

integrations/packages/elasticsearch/data_stream/node_stats/fields/package-fields.yml

Line 319 in b7bb08a

dimension: true

Can you check this commit to see what would make the most sense regarding other metric sets?
@consulthys
consulthys requested review from a team and consulthys and removed request for consulthys October 26, 2024 13:08
@consulthys
consulthys dismissed their stale review October 26, 2024 13:09

Asking for more checks

@BenB196

BenB196 commented Oct 26, 2024

Copy link
Copy Markdown
Contributor Author

Interestingly for the node_stats metric set, the elasticsearch.node.name was added as a dimension

integrations/packages/elasticsearch/data_stream/node_stats/fields/package-fields.yml

Line 319 in b7bb08a

dimension: true

Can you check this commit to see what would make the most sense regarding other metric sets?

Taking a look at the example provided, I think either would work, service.address being a dimension should prevent collisions between clusters if 2 clusters have the same elasticsearch.node.name value and are collected by the same centralized agent.

I've pushed a commit to use node.name here as with the other one.
@BenB196 BenB196 changed the title [Elasticsearch][Ingest Pipeline] Add elasticsearch.node.id as TSDS dimension Oct 26, 2024
@consulthys

consulthys commented Oct 26, 2024

Copy link
Copy Markdown
Contributor

Agreed, because elasticsearch.node.id is also not guaranteed to be unique across clusters, so either would be fine, but since other metric sets use elasticsearch.node.name it'd make sense to be consistent in this one also.
consulthys
consulthys approved these changes Oct 26, 2024
View reviewed changes

@consulthys consulthys left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM
@consulthys consulthys added Integration:elasticsearch Elasticsearch Feature:Stack Monitoring Stack Monitoring Feature Team:Stack Monitoring Stack Monitoring team [elastic/stack-monitoring] labels Oct 26, 2024
@andrewkroh andrewkroh added the bugfix Pull request that fixes a bug issue label Oct 28, 2024
@consulthys

consulthys commented Oct 31, 2024

Copy link
Copy Markdown
Contributor

/test
@elastic-vault-github-plugin-prod

elastic-vault-github-plugin-prod Bot commented Oct 31, 2024

Copy link
Copy Markdown
Contributor

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport
@elasticmachine

elasticmachine commented Oct 31, 2024

Copy link
Copy Markdown

💚 Build Succeeded
@elastic-sonarqube

elastic-sonarqube Bot commented Oct 31, 2024

Copy link
Copy Markdown

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube
@consulthys
consulthys merged commit a0663da into elastic:main Nov 1, 2024
@elastic-vault-github-plugin-prod

elastic-vault-github-plugin-prod Bot commented Nov 1, 2024

Copy link
Copy Markdown
Contributor

Package elasticsearch - 1.15.3 containing this change is available at https://epr.elastic.co/search?package=elasticsearch
@BenB196
BenB196 deleted the elasticsearch-ingest-pipeline-dimension-fix branch January 28, 2025 13:48
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 4, 2025
@BenB196
[Elasticsearch][Ingest Pipeline] Add elasticsearch.node.name as TSDS …
3013f87 
…dimension (elastic#11527)

* [Elasticsearch][Ingest Pipeline] Add elasticsearch.node.id as TSDS dimension

* Update changelog.yml

* switch from node id to node name
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 5, 2025
@BenB196
[Elasticsearch][Ingest Pipeline] Add elasticsearch.node.name as TSDS …
ff2bc8d 
…dimension (elastic#11527)

* [Elasticsearch][Ingest Pipeline] Add elasticsearch.node.id as TSDS dimension

* Update changelog.yml

* switch from node id to node name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Pull request that fixes a bug issue Feature:Stack Monitoring Stack Monitoring Feature Integration:elasticsearch Elasticsearch Team:Stack Monitoring Stack Monitoring team [elastic/stack-monitoring]

4 participants

@BenB196 @consulthys @elasticmachine @andrewkroh