Skip to content

Make elasticsearch/index metricset work for Stack Monitoring without xpack.enabled flag#21438

Merged
sayden merged 26 commits intoelastic:feature-stack-monitoring-mb-ecsfrom
sayden:feature/mb/elasticsearch/index_xpack_flag
Dec 1, 2020
Merged

Make elasticsearch/index metricset work for Stack Monitoring without xpack.enabled flag#21438
sayden merged 26 commits intoelastic:feature-stack-monitoring-mb-ecsfrom
sayden:feature/mb/elasticsearch/index_xpack_flag

Conversation

@sayden
Copy link
Copy Markdown
Contributor

@sayden sayden commented Sep 30, 2020

Ready to test with Kibana

Most of the fields will work with aliases but the fields that previously were on index_stats.* will now be in the same subpath but always under elasticsearch.index root path (and without index_stats at the beginning)

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Sep 30, 2020
@sayden sayden self-assigned this Sep 30, 2020
@sayden sayden added Metricbeat Metricbeat Feature:Stack Monitoring Team:Services (Deprecated) Label for the former Integrations-Services team labels Sep 30, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/stack-monitoring (Stack monitoring)

@elasticmachine
Copy link
Copy Markdown
Contributor

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

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Sep 30, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Sep 30, 2020

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #21438 updated

  • Start Time: 2020-12-01T12:53:13.089+0000

  • Duration: 61 min 3 sec

Test stats 🧪

Test Results
Failed 0
Passed 2244
Skipped 524
Total 2768

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 2244
Skipped 524
Total 2768

@chrisronline
Copy link
Copy Markdown
Contributor

chrisronline commented Oct 23, 2020

I'm seeing this error when running MB:

2020-10-23T13:59:06.418-0400 ERROR [publisher_pipeline_output] pipeline/output.go:154 Failed to connect to backoff(elasticsearch(https://localhost:9200)): Connection marked as failed because the onConnect callback failed: error loading template: could not load template. Elasticsearch returned: couldn't load template: 400 Bad Request: {"error":{"root_cause":[{"type":"mapper_parsing_exception","reason":"Invalid [path] value [elasticsearch.index.primaries.indexing.throttle_time.ms] for field alias [index_stats.primaries.indexing.throttle_time_in_millis]: an alias must refer to an existing field in the mappings."}],"type":"mapper_parsing_exception","reason":"Invalid [path] value [elasticsearch.index.primaries.indexing.throttle_time.ms] for field alias [index_stats.primaries.indexing.throttle_time_in_millis]: an alias must refer to an existing field in the mappings."},"status":400}. Response body: {"error":{"root_cause":[{"type":"mapper_parsing_exception","reason":"Invalid [path] value [elasticsearch.index.primaries.indexing.throttle_time.ms] for field alias [index_stats.primaries.indexing.throttle_time_in_millis]: an alias must refer to an existing field in the mappings."}],"type":"mapper_parsing_exception","reason":"Invalid [path] value [elasticsearch.index.primaries.indexing.throttle_time.ms] for field alias [index_stats.primaries.indexing.throttle_time_in_millis]: an alias must refer to an existing field in the mappings."},"status":400}

@sayden sayden changed the title Make beats/index metricset work for Stack Monitoring without xpack.enabled flag Make elasticsearch/index metricset work for Stack Monitoring without xpack.enabled flag Oct 28, 2020
@sayden
Copy link
Copy Markdown
Contributor Author

sayden commented Oct 28, 2020

Okay! Fixed, sorry for that. I still don't understand very well why those errors do not appear to me because this was tested with ES to produce the data.json.

Copy link
Copy Markdown
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

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

LGTM!

Screen Shot 2020-10-29 at 11 20 00 AM

Screen Shot 2020-10-29 at 11 20 07 AM

Screen Shot 2020-10-29 at 11 20 14 AM

@andresrc
Copy link
Copy Markdown
Contributor

@ycombinator I think this is ready for your final review, thanks!

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.

Consider replacing byt2 with something more descriptive or at least less arbitrary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good catch, I change its name to something more meaningful

Comment on lines 186 to 187
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.

I'm curious why these two fields need special treatment here. Shouldn't they get marshalled & unmarshalled above along with all the other fields?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Frankly, I don't remember either but I have double checked it and I think that it was an attempt to rename the fields to follow the naming conventions, that I later realized that I had way too many to change and then I regret and I forgot to remove those two.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I finally removed them

Copy link
Copy Markdown
Contributor Author

@sayden sayden Nov 3, 2020

Choose a reason for hiding this comment

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

Ha! No, forget that. They were original fields https://github.com/elastic/beats/pull/21438/files#diff-397cb07191b7b507fd7ca773b66d961954da4c2bfc5694ab8cf997e1cf991037R44

Instead of creating an alias I just duplicated those fields. I'll add the alias.

# Conflicts:
#	metricbeat/docs/fields.asciidoc
#	metricbeat/module/elasticsearch/fields.go
#	metricbeat/module/elasticsearch/node_stats/_meta/fields.yml
@sayden sayden force-pushed the feature/mb/elasticsearch/index_xpack_flag branch from f500817 to c0ed397 Compare November 5, 2020 20:47
@sayden sayden requested a review from ycombinator November 5, 2020 20:49
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 5, 2020

🐛 Flaky test report

❕ There are test failures but not known flaky tests.

Expand to view the summary

Test stats 🧪

Test Results
Failed 7
Passed 1958
Skipped 108
Total 2073

Genuine test errors 7

💔 There are test failures but not known flaky tests, most likely a genuine test failure.

  • Name: Build&Test / metricbeat-goIntegTest / TestGetAllIndices – elasticsearch
  • Name: Build&Test / metricbeat-unitTest / [build failed] – index
  • Name: Build&Test / metricbeat-windows-windows-2019 / [build failed] – index
  • Name: Build&Test / x-pack/metricbeat-build / test_export_index_pattern – x-pack.metricbeat.tests.system.test_xpack_base.Test
  • Name: Build&Test / x-pack/metricbeat-build / test_export_index_pattern_migration – x-pack.metricbeat.tests.system.test_xpack_base.Test
  • Name: Build&Test / x-pack/metricbeat-windows-windows-2019 / test_export_index_pattern – x-pack.metricbeat.tests.system.test_xpack_base.Test
  • Name: Build&Test / x-pack/metricbeat-windows-windows-2019 / test_export_index_pattern_migration – x-pack.metricbeat.tests.system.test_xpack_base.Test

Copy link
Copy Markdown
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

Left a minor comment about a TODO. Other than that, code changes LGTM.

However, tests are failing in CI and the failures look related to this PR. So please fix those before merging.

@sayden sayden force-pushed the feature/mb/elasticsearch/index_xpack_flag branch from c0ed397 to 0c0ded2 Compare November 10, 2020 16:45
@sayden sayden force-pushed the feature-stack-monitoring-mb-ecs branch from 58e2a7d to 9de5959 Compare November 12, 2020 11:44
…csearch/index_xpack_flag

# Conflicts:
#	metricbeat/docs/fields.asciidoc
#	metricbeat/module/elasticsearch/fields.go
#	metricbeat/module/elasticsearch/node_stats/_meta/fields.yml
…csearch/index_xpack_flag

# Conflicts:
#	metricbeat/docs/fields.asciidoc
#	metricbeat/module/elasticsearch/fields.go
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

…csearch/index_xpack_flag

# Conflicts:
#	metricbeat/module/elasticsearch/testing.go
…csearch/index_xpack_flag

# Conflicts:
#	metricbeat/docs/fields.asciidoc
#	metricbeat/module/elasticsearch/_meta/fields.yml
#	metricbeat/module/elasticsearch/fields.go
@sayden
Copy link
Copy Markdown
Contributor Author

sayden commented Nov 20, 2020

/test metricbeat

…csearch/index_xpack_flag

# Conflicts:
#	metricbeat/docs/fields.asciidoc
#	metricbeat/module/elasticsearch/fields.go
@sayden sayden merged commit ae189ad into elastic:feature-stack-monitoring-mb-ecs Dec 1, 2020
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Stack Monitoring Metricbeat Metricbeat Team:Services (Deprecated) Label for the former Integrations-Services team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants