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

Avoid generating incomplete configurations in autodiscover #20898

Merged
merged 5 commits into from
Sep 2, 2020

Conversation

jsoriano
Copy link
Member

@jsoriano jsoriano commented Sep 1, 2020

What does this PR do?

Handle errors when configuration unpacking fails. In principle this can
only happen when some variable is missing, because configuration has
been previously parsed as YAML. Errors on unpacking were previously
ignored.

When a variable is missing, this is clearly logged at the debug level.

This changes the behaviour, previously an incomplete configuration was
generated on this case.

Why is it important?

To detect problems with incomplete configuration earlier and improve feedback in logs
when a variable is missing in event.

Since #18979 this error is more frequent, as a legit configuration for containers produces
errors with pod events that don't contain container fields.

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.

Related issues

Logs

For example the following configuration includes an incorrect field container.id.

filebeat.autodiscover:
  providers:
    - type: docker
      hints.enabled: true
      hints.default_config:
        type: container
        paths:
          - /var/lib/docker/containers/${container.id}/*.log

Before this change, an incomplete configuration is generated, and errors like these ones are logged:

2020-09-01T16:52:50.094+0200	ERROR	[autodiscover]	autodiscover/autodiscover.go:209	Auto discover config check failed for config '{
  "docker-json": {
    "cri_flags": true,
    "format": "auto",
    "partial": true,
    "stream": "all"
  },
  "symlinks": true,
  "type": "container"
}', won't start runner: each input must have at least one path defined

After this change, the cause of the problem is more clear, and it is only logged at the debug level:

2020-09-02T11:57:08.225+0200	DEBUG	[autodiscover]	template/config.go:156	Configuration template cannot be resolved: field 'container.id' not available in event or environment accessing 'paths' (source:'/home/jaime/tmp/filebeat-autodiscover-docker-hints.yml')

Handle errors when configuration unpacking fails. In principle this can
only happen when some variable is missing, because configuration has
been previously parsed as YAML. Errors on unpacking were previously
ignored.

When a variable is missing, this is clearly logged at the debug level.

This changes the behaviour, previously an incomplete configuration was
generated on this case.
@jsoriano jsoriano added review Team:Platforms Label for the Integrations - Platforms team labels Sep 1, 2020
@jsoriano jsoriano requested a review from ChrsMark September 1, 2020 15:05
@jsoriano jsoriano self-assigned this Sep 1, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-platforms (Team:Platforms)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Sep 1, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 1, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #20898 updated]

  • Start Time: 2020-09-02T09:54:23.224+0000

  • Duration: 66 min 4 sec

Test stats 🧪

Test Results
Failed 0
Passed 19661
Skipped 1830
Total 21491

@jsoriano jsoriano added needs_backport PR is waiting to be backported to other branches. v7.10.0 labels Sep 1, 2020
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.

Looks good to me! Thanks for fixing this! I just left a nit question.

libbeat/autodiscover/template/config.go Outdated Show resolved Hide resolved
@jsoriano jsoriano merged commit 99fd545 into elastic:master Sep 2, 2020
@jsoriano jsoriano deleted the autodiscover-unpack-config-error branch September 2, 2020 11:15
jsoriano added a commit to jsoriano/beats that referenced this pull request Sep 2, 2020
…0898)

Handle errors when configuration unpacking fails. In principle this can
only happen when some variable is missing, because configuration has
been previously parsed as YAML. Errors on unpacking were previously
ignored.

When a variable is missing, this is clearly logged at the debug level.

This changes the behaviour, previously an incomplete configuration was
generated on this case.

(cherry picked from commit 99fd545)
@jsoriano jsoriano removed the needs_backport PR is waiting to be backported to other branches. label Sep 2, 2020
jsoriano added a commit to jsoriano/beats that referenced this pull request Sep 2, 2020
…0898)

Handle errors when configuration unpacking fails. In principle this can
only happen when some variable is missing, because configuration has
been previously parsed as YAML. Errors on unpacking were previously
ignored.

When a variable is missing, this is clearly logged at the debug level.

This changes the behaviour, previously an incomplete configuration was
generated on this case.

(cherry picked from commit 99fd545)
@jsoriano jsoriano added the v7.9.1 label Sep 2, 2020
v1v added a commit to v1v/beats that referenced this pull request Sep 2, 2020
…ne-2.0

* upstream/master: (87 commits)
  [packaging] Normalise GCP bucket folder structure (elastic#20903)
  [Metricbeat] Add billing metricset into googlecloud module (elastic#20812)
  Include python docs in devguide index (elastic#20917)
  Avoid generating incomplete configurations in autodiscover (elastic#20898)
  Improve docs of leaderelection configuration (elastic#20601)
  Document how to set the ES host and Kibana URLs in Ingest Manager (elastic#20874)
  docs: Update beats for APM (elastic#20881)
  Adding cborbeat to community beats (elastic#20884)
  Bump kibana version to 7.9.0 in x-pack/metricbeat (elastic#20899)
  Kubernetes state_daemonset metricset for Metricbeat (elastic#20649)
  [Filebeat][zeek] Add new x509 fields to zeek (elastic#20867)
  [Filebeat][Gsuite] Add note about admin in gsuite docs (elastic#20855)
  Ensure kind cluster has RFC1123 compliant name (elastic#20627)
  Setup python paths in test runner configuration (elastic#20832)
  docs: Add  `processor.event` info to Logstash output (elastic#20721)
  docs: update cipher suites (elastic#20697)
  [ECS] Update ecs to 1.6.0 (elastic#20792)
  Fix path in hits docs (elastic#20447)
  Update filebeat azure module documentation (elastic#20815)
  Remove duplicate ListGroupsForUsers in aws/cloudtrail (elastic#20788)
  ...
jsoriano added a commit that referenced this pull request Sep 3, 2020
… in autodiscover (#20919)

Handle errors when configuration unpacking fails. In principle this can
only happen when some variable is missing, because configuration has
been previously parsed as YAML. Errors on unpacking were previously
ignored.

When a variable is missing, this is clearly logged at the debug level.

This changes the behaviour, previously an incomplete configuration was
generated on this case.

(cherry picked from commit 99fd545)
@jsoriano jsoriano removed the v7.9.1 label Sep 3, 2020
@jsoriano jsoriano added the v7.9.2 label Sep 3, 2020
jsoriano added a commit that referenced this pull request Sep 4, 2020
… in autodiscover (#20920)

Handle errors when configuration unpacking fails. In principle this can
only happen when some variable is missing, because configuration has
been previously parsed as YAML. Errors on unpacking were previously
ignored.

When a variable is missing, this is clearly logged at the debug level.

This changes the behaviour, previously an incomplete configuration was
generated on this case.

(cherry picked from commit 99fd545)
melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
…0898)

Handle errors when configuration unpacking fails. In principle this can
only happen when some variable is missing, because configuration has
been previously parsed as YAML. Errors on unpacking were previously
ignored.

When a variable is missing, this is clearly logged at the debug level.

This changes the behaviour, previously an incomplete configuration was
generated on this case.
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…rations in autodiscover (elastic#20920)

Handle errors when configuration unpacking fails. In principle this can
only happen when some variable is missing, because configuration has
been previously parsed as YAML. Errors on unpacking were previously
ignored.

When a variable is missing, this is clearly logged at the debug level.

This changes the behaviour, previously an incomplete configuration was
generated on this case.

(cherry picked from commit ff60b11)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Team:Platforms Label for the Integrations - Platforms team v7.9.2 v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve logging when autodiscover configs fail
3 participants