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

Monitoring: only include X-Pack API params when appropriate #18787

Merged
merged 2 commits into from
May 27, 2020

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented May 27, 2020

What does this PR do?

This PR fixes the Elasticsearch monitoring reporter in libbeat to only include the system_id and system_api_version query string parameters in Elasticsearch API requests IF the Beat is sending its monitoring data to the custom X-Pack Monitoring API endpoint (/_monitoring/bulk). Otherwise, these parameters are not included.

Why is it important?

A Beat can be configured to send its monitoring data to Elasticsearch in two ways:

  • using the deprecated xpack.monitoring.* settings. This causes the Beat to talk to the custom X-Pack Monitoring API endpoint (/_monitoring/bulk). This endpoint expects the system_id and system_api_version query string parameters.
  • using the monitoring.* settings. This causes the Beat to talk to the generic Bulk API endpoint (/_bulk). This endpoint does not expect the system_id or system_api_version query string parameters.

It turns out that, after the fix made in #18326, the Elasticsearch monitoring reporter in libbeat was always sending the system_id and system_api_version query string parameters, regardless of which way a Beat was sending its monitoring data to Elasticsearch. This PR fixes this issue.

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. Not sure we need a CHANGELOG entry as this bug was not exposed until 7.8.0 (via [Libbeat] Respect the parameters option defined in the ES output. #18326), which has not yet been released.

Related issues

@ycombinator ycombinator added bug needs_backport PR is waiting to be backported to other branches. Team:Integrations Label for the Integrations team v8.0.0 v7.8.0 labels May 27, 2020
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label May 27, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label May 27, 2020
@ycombinator ycombinator requested review from urso and exekias May 27, 2020 18:27
@ycombinator ycombinator marked this pull request as ready for review May 27, 2020 18:28
@elasticmachine
Copy link
Collaborator

elasticmachine commented May 27, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #18787 updated]

  • Start Time: 2020-05-27T18:27:09.839+0000

  • Duration: 76 min 37 sec

Test stats 🧪

Test Results
Failed 0
Passed 9242
Skipped 1542
Total 10784

Steps errors

Expand to view the steps failures

  • 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: 1 min 27 sec

    • Start Time: 2020-05-27T19:05:03.619+0000

    • log

  • Name: Make -C generator/_templates/metricbeat test

    • Description: make -C generator/_templates/metricbeat test

    • Duration: 7 min 2 sec

    • Start Time: 2020-05-27T19:25:43.282+0000

    • log

@ycombinator ycombinator requested a review from ph May 27, 2020 18:33
Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

LGTM

@ycombinator ycombinator merged commit 76dd79f into elastic:master May 27, 2020
@ycombinator ycombinator deleted the lb-mon-params branch May 27, 2020 19:49
@ycombinator ycombinator added v7.9.0 and removed needs_backport PR is waiting to be backported to other branches. labels May 27, 2020
ycombinator added a commit that referenced this pull request May 27, 2020
…18793)

* Only include X-Pack API params if we're going to be talking to the X-Pack API!

* Refactoring for testability
ycombinator added a commit that referenced this pull request May 27, 2020
…18794)

* Only include X-Pack API params if we're going to be talking to the X-Pack API!

* Refactoring for testability
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to send monitoring data due to 400 error from Elasticsearch bulk API
5 participants