Skip to content

Avoid generating incomplete configurations in autodiscover#20898

Merged
jsoriano merged 5 commits intoelastic:masterfrom
jsoriano:autodiscover-unpack-config-error
Sep 2, 2020
Merged

Avoid generating incomplete configurations in autodiscover#20898
jsoriano merged 5 commits intoelastic:masterfrom
jsoriano:autodiscover-unpack-config-error

Conversation

@jsoriano
Copy link
Copy Markdown
Member

@jsoriano jsoriano commented Sep 1, 2020 •
edited
Loading

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')

@jsoriano
Avoid generating incomplete configurations in autodiscover
8f5e7e4 
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
Copy Markdown
Contributor

elasticmachine commented Sep 1, 2020

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
Copy Markdown
Contributor

elasticmachine commented Sep 1, 2020 •
edited by jenkins-beats-ci bot
Loading

💚 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
ChrsMark
ChrsMark reviewed Sep 2, 2020
View reviewed changes
Copy link
Copy Markdown
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.

@ChrsMark ChrsMark mentioned this pull request Sep 2, 2020
Merged
@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
@jsoriano
Avoid generating incomplete configurations in autodiscover (elastic#2…
74cd90b 
…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 mentioned this pull request Sep 2, 2020
Merged
6 tasks
@jsoriano jsoriano removed the needs_backport PR is waiting to be backported to other branches. label Sep 2, 2020
@jsoriano jsoriano mentioned this pull request Sep 2, 2020
Merged
6 tasks
jsoriano added a commit to jsoriano/beats that referenced this pull request Sep 2, 2020
@jsoriano
Avoid generating incomplete configurations in autodiscover (elastic#2…
63eb064 
…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
@ChrsMark ChrsMark mentioned this pull request Sep 2, 2020
Merged
v1v added a commit to v1v/beats that referenced this pull request Sep 2, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/ci-pipeli…
1faa11d 
…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
@jsoriano
Cherry-pick #20898 to 7.x: Avoid generating incomplete configurations…
19a650f 
… 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
@jsoriano
Cherry-pick #20898 to 7.9: Avoid generating incomplete configurations…
5be9bcd 
… 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
@jsoriano @melchiormoulin
Avoid generating incomplete configurations in autodiscover (elastic#2…
1469c48 
…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
@jsoriano
Cherry-pick elastic#20898 to 7.9: Avoid generating incomplete configu…
c1f61db 
…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

Reviewers

@ChrsMark ChrsMark ChrsMark approved these changes

Assignees

@jsoriano jsoriano

Labels

review Team:Platforms Label for the Integrations - Platforms team v7.9.2 v7.10.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve logging when autodiscover configs fail

4 participants

@jsoriano @elasticmachine @ChrsMark @andresrc