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

Refactor use of system.hostfs to fix cgroup metrics #24334

Merged

Conversation

fearful-symmetry
Copy link
Contributor

What does this PR do?

This is a fix for #23267, wherein we weren't passing a hostfs to the metricbeat self-monitor code. In order to fix this in a somewhat clean way, I ended up migrating all of the hostfs handling to libbeat/path, and out of the system module entirely. This is a bit of a weird edge case for libbeat/path, so just tell me if this should be done somewhat differently.

Why is it important?

As explained in the linked issue, this is causing problems for users setting hostfs.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

  • Pull down, build.
  • Make sure the socket, process,entropy and diskio are working correctly.
  • Run metricbeat with an alternate system.hostfs setting, such as in a container, like this:
docker run --mount type=bind,source=/proc,target=/hostfs/proc,readonly --mount type=bind,source=/sys/fs/cgroup,target=/hostfs/sys/fs/cgroup,readonly --mount type=bind,source=/,target=/hostfs,readonly --net=host docker.elastic.co/beats/metricbeat:8.0.0 -e -system.hostfs=/hostfs -E 'output.elasticsearch.hosts=["localhost:9200"]'

Insure that the above metricsets are still working, and that no errors like instance/metrics.go:285 error getting group status: open /proc/33861/cgroup: no such file or directory appear.

Related issues

Logs

Tested, it looks like we're properly reporting cgroup metrics inside a docker container:

2021-03-03T21:42:58.879Z        INFO    [monitoring]    log/log.go:152  Total non-zero metrics  {"monitoring": {"metrics": {"beat":{"cgroup":{"cpu":{"cfs":{"period":{"us":100000}},"id":"2716b9eb8166589954bcd5e6e8c73c3f08913e1604d60e27bb0f631014a5789e"},"cpuacct":{"id":"2716b9eb8166589954bcd5e6e8c73c3f08913e1604d60e27bb0f631014a5789e","total":{"ns":8121549013889171}},"memory":{"id":"2716b9eb8166589954bcd5e6e8c73c3f08913e1604d60e27bb0f631014a5789e","mem":{"limit":{"bytes":9223372036854771712},"usage":{"bytes":11272822784}}}},"cpu":{"system":{"ticks":7910,"time":{"ms":7911}},"total":{"ticks":14340,"time":{"ms":14347},"value":14340},"user":{"ticks":6430,"time":{"ms":6436}}},"handles":{"limit":{"hard":1048576,"soft":1048576},"open":10},"info":{"ephemeral_id":"c3361a23-ba63-4044-8926-e3e188e48715","uptime":{"ms":270554}},"memstats":{"gc_next":16976352,"memory_alloc":9944136,"memory_sys":75580416,"memory_total":484400016,"rss":86827008},"runtime":{"goroutines":16}},"libbeat":{"config":{"module":{"running":3,"starts":3},"reloads":1,"scans":1},"output":{"events":{"acked":628,"active":0,"batches":53,"total":628},"read":{"bytes":25316},"type":"elasticsearch","write":{"bytes":1123236}},"pipeline":{"clients":0,"events":{"active":4,"published":632,"retry":27,"total":632},"queue":{"acked":628}}},"metricbeat":{"system":{"cpu":{"events":28,"success":28},"filesystem":{"events":10,"success":10},"fsstat":{"events":5,"success":5},"load":{"events":28,"success":28},"memory":{"events":28,"success":28},"network":{"events":242,"success":242},"process":{"events":235,"success":235},"process_summary":{"events":27,"success":28},"socket_summary":{"events":28,"success":28},"uptime":{"events":1,"success":1}}},"system":{"cpu":{"cores":4},"load":{"1":3.76,"15":3.76,"5":3.54,"norm":{"1":0.94,"15":0.94,"5":0.885}}}}}}

@fearful-symmetry fearful-symmetry requested review from urso and a team March 3, 2021 23:10
@fearful-symmetry fearful-symmetry self-assigned this Mar 3, 2021
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Mar 3, 2021
@fearful-symmetry fearful-symmetry added the Team:Integrations Label for the Integrations team label Mar 3, 2021
@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 Mar 3, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Mar 4, 2021

💚 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 #24334 updated

  • Start Time: 2021-03-11T15:38:25.736+0000

  • Duration: 133 min 39 sec

  • Commit: c9acdb9

Test stats 🧪

Test Results
Failed 0
Passed 45209
Skipped 4990
Total 50199

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 45209
Skipped 4990
Total 50199

@ChrsMark
Copy link
Member

ChrsMark commented Mar 4, 2021

Maybe to add this option to https://github.com/elastic/beats/blob/9735c6d51540b2fb72abfbbea0a165c033619542/libbeat/docs/shared-path-config.asciidoc too?

Also in the manual testing section would be nice to check if setting the path through the configuration file works too.

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Nice. Added some questions, mainly about the name of the option, and its availability for other beats.

libbeat/cmd/instance/beat.go Show resolved Hide resolved
@@ -28,6 +28,8 @@
//
// path.config - Configuration files and Elasticsearch template default location
//
// system.hostfs - supplies an alternate filesystem root for containerized environments
Copy link
Member

Choose a reason for hiding this comment

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

Wdyt about calling it path.hostfs now that it is another configurable path, and not only used by the system module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yah, it might be worth it to have a second one for backwards compatibility?

Copy link
Member

Choose a reason for hiding this comment

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

Yep, we would keep both at least till 8.0.

libbeat/paths/paths.go Show resolved Hide resolved
metricbeat/module/system/entropy/entropy_test.go Outdated Show resolved Hide resolved
CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
}{}

if err := cfg.Unpack(&partialConfig); err != nil {
return fmt.Errorf("error extracting default paths: %+v", err)
}

partialConfig.Path.Hostfs = partialConfig.Hostfs
Copy link

Choose a reason for hiding this comment

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

Is this always safe? By introducing Hostfs to the Path struct we will allow users to overwrite the path in the configuration file already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I'm missing something, but is that any different from -E flags?

Copy link

Choose a reason for hiding this comment

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

Paths are configured twice I think. Once via --path.X on startup + --system.hostfs.

Then the config file is read and the path settings from the configuration file are applied again. The -E flags for path settings only become effective during the second phase. That is why the shim bash scripts just use --path.X CLI flags and we do not attempt to overwrite the defaults in the config.

The thing I was wondering: What if path.hostfs is configured, but system.hostfs is not? In that case you will set path.hostfs to an empty string here. Is this on purpose? Why not:

if partialConfig.Path.Hostfs == "" {
  partialConfig.Path.Hostfs = partialConfig.Hostfs
}

If the forced overwrite is on purpose a comment explaining the why would be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, so, technically speaking, there is no path.hostfs. There's only system.hostfs, and we just expand the main Path struct type with hostfs, hence why that's being overwritten. Ideally it would only be path.hostfs, but I'm mostly worried about breaking changes here, since I'm considering this to be primarily a bugfix so we can properly set hostfs in the cgroup self-monitoring.

Copy link

Choose a reason for hiding this comment

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

I see. It is still kinda "obscure" and I'm sure someone (me?, future you?) might change the logic on purpose or by accident. Can you please add a code comment about the why? Having the why in the PR review it will be lost.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good point.

@fearful-symmetry
Copy link
Contributor Author

Bah, circular dependency with the cfgwarn import.

@fearful-symmetry
Copy link
Contributor Author

Alright, I think I should probably put the hostfs flag declaration somewhere else. Just not sure where...

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

LGTM to fix #23267 in Metricbeat.

I still wonder if the setting in the config file should be called path.hostfs instead of system.hostfs, specially if this can be used on other beats, where the path prefix would make more sense to me than the system one. But this could be done in a future change, and we could support both names during a time for compatibility with Metricbeat.

Comment on lines +111 to +115
==== `system.hostfs`

Specifies the mount point of the host's filesystem for use in monitoring a host.
This can either be set in the config, or with the `--system.hostfs` CLI flag. This is used for cgroup self-monitoring.
ifeval::["{beatname_lc}"=="metricbeat"]
Copy link
Member

Choose a reason for hiding this comment

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

If this setting is used for other beats too, I think we could add it as path.hostfs for the config file setting, as other beats don't have a system module. But we can leave it for a future change.

@@ -28,6 +28,8 @@
//
// path.config - Configuration files and Elasticsearch template default location
//
// system.hostfs - supplies an alternate filesystem root for containerized environments
Copy link
Member

Choose a reason for hiding this comment

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

Yep, we would keep both at least till 8.0.

@jsoriano
Copy link
Member

/test

@urso
Copy link

urso commented Mar 11, 2021

/test

@fearful-symmetry
Copy link
Contributor Author

Yah, I have no idea what's wrong with CI here.

@fearful-symmetry fearful-symmetry merged commit c70fe5c into elastic:master Mar 11, 2021
fearful-symmetry added a commit to fearful-symmetry/beats that referenced this pull request Mar 11, 2021
* refactor use of system.hostfs to fix cgroup metrics

* add changelog

* remove comment

* add cfgwarn

* move changelog

* shift around CLI config location and rep warning

* add comment about system.hostfs usage

* update docs

* capitalization

* fix grammar, add conditional

* change docs phrasing

(cherry picked from commit c70fe5c)
@schans
Copy link

schans commented Mar 15, 2021

Hi!

I noticed the issue has the v7.11.2 label but is not part of the release as far as I can tell. Is v7.13.0 going to be the next release after v7.11.x? If so, when is that to be expected?

fearful-symmetry added a commit that referenced this pull request Mar 15, 2021
…p metrics (#24517)

* Refactor use of system.hostfs to fix cgroup metrics (#24334)

* refactor use of system.hostfs to fix cgroup metrics

* add changelog

* remove comment

* add cfgwarn

* move changelog

* shift around CLI config location and rep warning

* add comment about system.hostfs usage

* update docs

* capitalization

* fix grammar, add conditional

* change docs phrasing

(cherry picked from commit c70fe5c)

* fix changelog
narph pushed a commit to narph/beats that referenced this pull request Mar 15, 2021
* refactor use of system.hostfs to fix cgroup metrics

* add changelog

* remove comment

* add cfgwarn

* move changelog

* shift around CLI config location and rep warning

* add comment about system.hostfs usage

* update docs

* capitalization

* fix grammar, add conditional

* change docs phrasing
v1v added a commit to v1v/beats that referenced this pull request Mar 16, 2021
…reporting-changes

* upstream/master: (23 commits)
  [Auditbeat] btmp offset check (elastic#24515)
  Clarify that the Tomcat module is for ingesting access logs (elastic#24543)
  [Ingest Manager] Move logging defaults to agent (elastic#24535)
  Update input-http-endpoint.asciidoc (elastic#24490)
  Fix typo in mqtt input docs (elastic#24509)
  [Elastic Agent] Add verification check when updating communication to Kibana. (elastic#24489)
  Refactor use of system.hostfs to fix cgroup metrics (elastic#24334)
  Add test for close.reader.after_interval to filestream input (elastic#24423)
  chore(ci): use beat_version instead of PR version (elastic#24446)
  Add syntax for multiple selector logging (elastic#24207) (elastic#24497)
  Update Golang to 1.15.9 (elastic#24442)
  [Elastic Agent] Add the ability to provide custom CA's inside of docker. (elastic#24486)
  Add tests for encoding settings of filestream input (elastic#24426)
  [Ingest Manager] Sync on rename on windows (elastic#24504)
  Port four Harvester tests of log input to filestream in Golang (elastic#24250)
  [DOCS] Restructure content for SSL settings (elastic#24342)
  Move example to the correct location in reference docs (elastic#24455)
  Add unit tests for harvester.go of input-logfile package (elastic#24107)
  Fix type for uwsgi.status.worker.rss field (elastic#24468)
  [Ingest Manager] Logging to file disabled on enroll (elastic#24466)
  ...
@fearful-symmetry
Copy link
Contributor Author

@schans this ended up being a more complicated change than I expected, so we missed 7.11.2. @urso I don't see anything that would theoretically prevent us from backporting to 7.12?

narph added a commit that referenced this pull request Mar 18, 2021
* mofidy doc

* work on fix

* update changelog

* update notice

* Update notice

* [Ingest Manager] Sync on rename on windows (#24504)

* Add tests for encoding settings of filestream input (#24426)

* [Elastic Agent] Add the ability to provide custom CA's inside of docker. (#24486)

* Add the ability to provide custom CA's for Elastic Agent docker.

* Add changelog.

* Update Golang to 1.15.9 (#24442)

* Add syntax for multiple selector logging (#24207) (#24497)

* Add syntax for multiple selector logging

* Update libbeat/docs/loggingconfig.asciidoc

Co-authored-by: EamonnTP <[email protected]>

Co-authored-by: EamonnTP <[email protected]>

Co-authored-by: AndyHunt66 <[email protected]>

* chore(ci): use beat_version instead of PR version (#24446)

* Add test for close.reader.after_interval to filestream input (#24423)

* Refactor use of system.hostfs to fix cgroup metrics (#24334)

* refactor use of system.hostfs to fix cgroup metrics

* add changelog

* remove comment

* add cfgwarn

* move changelog

* shift around CLI config location and rep warning

* add comment about system.hostfs usage

* update docs

* capitalization

* fix grammar, add conditional

* change docs phrasing

* [Elastic Agent] Add verification check when updating communication to Kibana. (#24489)

* Add verification check when updating communication to Kibana.

* Add changelog.

* Add const.

* Fix typo in mqtt input docs (#24509)

* Update input-http-endpoint.asciidoc (#24490)

* [Ingest Manager] Move logging defaults to agent (#24535)

[Ingest Manager] Move logging defaults to agent (#24535)

* Clarify that the Tomcat module is for ingesting access logs (#24543)

The Tomcat module is for ingesting access logs, not Catalina or localhost logs.

* [Auditbeat] btmp offset check (#24515)

* auditbeat btmp offset check

Add check that saved offset is not larger than the current file size
to prevent seeking past the end of file

* [Heartbeat] Produce error rather than panic on missing source (#24404)

Fixes #24403.

With the changes to the heartbeat config syntax in 7.12 the `source`
field is now required. Our config validation code didn't actually check
for this field's presence, which caused an NPE.

This PR adds a validation checking for that config's presence. It also
adds tests for the validation code for config sub-fields. There were no
defects found in the validations for source.inline, or source.browser,
but a few tests were missing.

Instead of the panic seen in #24403 users will now get the error seen
below.

```
2021-03-05T15:41:40.146-0600	ERROR	instance/beat.go:952	Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
```

* Fix default scope in add_nomad_metadata (#24559)

Fix default scope in add_nomad_metadata. It is set to local, but
it should be node. Fix also error message that showed that local
is a valid value.

* [Filebeat] Add Dashboards to Threat Intel Module (#24488)

* added dashboards & docs

* ran mage fmt update

* [CI] bump gvm version and use the binary (#24571)

* [CI] Add resilience when installing required tooling (#24542)

* [CI] enable new flaky detector (#24464)

* chore: do not pass beat version (#24586)

We will be delegating the version calculation to the e2e tests, using
target branch values as defaults

Co-authored-by: Jaime Soriano Pastor <[email protected]>
Co-authored-by: Michal Pristas <[email protected]>
Co-authored-by: Noémi Ványi <[email protected]>
Co-authored-by: Blake Rouse <[email protected]>
Co-authored-by: EamonnTP <[email protected]>
Co-authored-by: AndyHunt66 <[email protected]>
Co-authored-by: Manuel de la Peña <[email protected]>
Co-authored-by: Alex K <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: Lee Hinman <[email protected]>
Co-authored-by: Andrew Cholakian <[email protected]>
Co-authored-by: Andrew Pease <[email protected]>
Co-authored-by: Victor Martinez <[email protected]>
narph added a commit to narph/beats that referenced this pull request Mar 18, 2021
* mofidy doc

* work on fix

* update changelog

* update notice

* Update notice

* [Ingest Manager] Sync on rename on windows (elastic#24504)

* Add tests for encoding settings of filestream input (elastic#24426)

* [Elastic Agent] Add the ability to provide custom CA's inside of docker. (elastic#24486)

* Add the ability to provide custom CA's for Elastic Agent docker.

* Add changelog.

* Update Golang to 1.15.9 (elastic#24442)

* Add syntax for multiple selector logging (elastic#24207) (elastic#24497)

* Add syntax for multiple selector logging

* Update libbeat/docs/loggingconfig.asciidoc

Co-authored-by: EamonnTP <[email protected]>

Co-authored-by: EamonnTP <[email protected]>

Co-authored-by: AndyHunt66 <[email protected]>

* chore(ci): use beat_version instead of PR version (elastic#24446)

* Add test for close.reader.after_interval to filestream input (elastic#24423)

* Refactor use of system.hostfs to fix cgroup metrics (elastic#24334)

* refactor use of system.hostfs to fix cgroup metrics

* add changelog

* remove comment

* add cfgwarn

* move changelog

* shift around CLI config location and rep warning

* add comment about system.hostfs usage

* update docs

* capitalization

* fix grammar, add conditional

* change docs phrasing

* [Elastic Agent] Add verification check when updating communication to Kibana. (elastic#24489)

* Add verification check when updating communication to Kibana.

* Add changelog.

* Add const.

* Fix typo in mqtt input docs (elastic#24509)

* Update input-http-endpoint.asciidoc (elastic#24490)

* [Ingest Manager] Move logging defaults to agent (elastic#24535)

[Ingest Manager] Move logging defaults to agent (elastic#24535)

* Clarify that the Tomcat module is for ingesting access logs (elastic#24543)

The Tomcat module is for ingesting access logs, not Catalina or localhost logs.

* [Auditbeat] btmp offset check (elastic#24515)

* auditbeat btmp offset check

Add check that saved offset is not larger than the current file size
to prevent seeking past the end of file

* [Heartbeat] Produce error rather than panic on missing source (elastic#24404)

Fixes elastic#24403.

With the changes to the heartbeat config syntax in 7.12 the `source`
field is now required. Our config validation code didn't actually check
for this field's presence, which caused an NPE.

This PR adds a validation checking for that config's presence. It also
adds tests for the validation code for config sub-fields. There were no
defects found in the validations for source.inline, or source.browser,
but a few tests were missing.

Instead of the panic seen in elastic#24403 users will now get the error seen
below.

```
2021-03-05T15:41:40.146-0600	ERROR	instance/beat.go:952	Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
```

* Fix default scope in add_nomad_metadata (elastic#24559)

Fix default scope in add_nomad_metadata. It is set to local, but
it should be node. Fix also error message that showed that local
is a valid value.

* [Filebeat] Add Dashboards to Threat Intel Module (elastic#24488)

* added dashboards & docs

* ran mage fmt update

* [CI] bump gvm version and use the binary (elastic#24571)

* [CI] Add resilience when installing required tooling (elastic#24542)

* [CI] enable new flaky detector (elastic#24464)

* chore: do not pass beat version (elastic#24586)

We will be delegating the version calculation to the e2e tests, using
target branch values as defaults

Co-authored-by: Jaime Soriano Pastor <[email protected]>
Co-authored-by: Michal Pristas <[email protected]>
Co-authored-by: Noémi Ványi <[email protected]>
Co-authored-by: Blake Rouse <[email protected]>
Co-authored-by: EamonnTP <[email protected]>
Co-authored-by: AndyHunt66 <[email protected]>
Co-authored-by: Manuel de la Peña <[email protected]>
Co-authored-by: Alex K <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: Lee Hinman <[email protected]>
Co-authored-by: Andrew Cholakian <[email protected]>
Co-authored-by: Andrew Pease <[email protected]>
Co-authored-by: Victor Martinez <[email protected]>
(cherry picked from commit 5c6f1b6)
narph added a commit to narph/beats that referenced this pull request Mar 18, 2021
* mofidy doc

* work on fix

* update changelog

* update notice

* Update notice

* [Ingest Manager] Sync on rename on windows (elastic#24504)

* Add tests for encoding settings of filestream input (elastic#24426)

* [Elastic Agent] Add the ability to provide custom CA's inside of docker. (elastic#24486)

* Add the ability to provide custom CA's for Elastic Agent docker.

* Add changelog.

* Update Golang to 1.15.9 (elastic#24442)

* Add syntax for multiple selector logging (elastic#24207) (elastic#24497)

* Add syntax for multiple selector logging

* Update libbeat/docs/loggingconfig.asciidoc

Co-authored-by: EamonnTP <[email protected]>

Co-authored-by: EamonnTP <[email protected]>

Co-authored-by: AndyHunt66 <[email protected]>

* chore(ci): use beat_version instead of PR version (elastic#24446)

* Add test for close.reader.after_interval to filestream input (elastic#24423)

* Refactor use of system.hostfs to fix cgroup metrics (elastic#24334)

* refactor use of system.hostfs to fix cgroup metrics

* add changelog

* remove comment

* add cfgwarn

* move changelog

* shift around CLI config location and rep warning

* add comment about system.hostfs usage

* update docs

* capitalization

* fix grammar, add conditional

* change docs phrasing

* [Elastic Agent] Add verification check when updating communication to Kibana. (elastic#24489)

* Add verification check when updating communication to Kibana.

* Add changelog.

* Add const.

* Fix typo in mqtt input docs (elastic#24509)

* Update input-http-endpoint.asciidoc (elastic#24490)

* [Ingest Manager] Move logging defaults to agent (elastic#24535)

[Ingest Manager] Move logging defaults to agent (elastic#24535)

* Clarify that the Tomcat module is for ingesting access logs (elastic#24543)

The Tomcat module is for ingesting access logs, not Catalina or localhost logs.

* [Auditbeat] btmp offset check (elastic#24515)

* auditbeat btmp offset check

Add check that saved offset is not larger than the current file size
to prevent seeking past the end of file

* [Heartbeat] Produce error rather than panic on missing source (elastic#24404)

Fixes elastic#24403.

With the changes to the heartbeat config syntax in 7.12 the `source`
field is now required. Our config validation code didn't actually check
for this field's presence, which caused an NPE.

This PR adds a validation checking for that config's presence. It also
adds tests for the validation code for config sub-fields. There were no
defects found in the validations for source.inline, or source.browser,
but a few tests were missing.

Instead of the panic seen in elastic#24403 users will now get the error seen
below.

```
2021-03-05T15:41:40.146-0600	ERROR	instance/beat.go:952	Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
```

* Fix default scope in add_nomad_metadata (elastic#24559)

Fix default scope in add_nomad_metadata. It is set to local, but
it should be node. Fix also error message that showed that local
is a valid value.

* [Filebeat] Add Dashboards to Threat Intel Module (elastic#24488)

* added dashboards & docs

* ran mage fmt update

* [CI] bump gvm version and use the binary (elastic#24571)

* [CI] Add resilience when installing required tooling (elastic#24542)

* [CI] enable new flaky detector (elastic#24464)

* chore: do not pass beat version (elastic#24586)

We will be delegating the version calculation to the e2e tests, using
target branch values as defaults

Co-authored-by: Jaime Soriano Pastor <[email protected]>
Co-authored-by: Michal Pristas <[email protected]>
Co-authored-by: Noémi Ványi <[email protected]>
Co-authored-by: Blake Rouse <[email protected]>
Co-authored-by: EamonnTP <[email protected]>
Co-authored-by: AndyHunt66 <[email protected]>
Co-authored-by: Manuel de la Peña <[email protected]>
Co-authored-by: Alex K <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: Lee Hinman <[email protected]>
Co-authored-by: Andrew Cholakian <[email protected]>
Co-authored-by: Andrew Pease <[email protected]>
Co-authored-by: Victor Martinez <[email protected]>
(cherry picked from commit 5c6f1b6)
narph added a commit that referenced this pull request Mar 26, 2021
* mofidy doc

* work on fix

* update changelog

* update notice

* Update notice

* [Ingest Manager] Sync on rename on windows (#24504)

* Add tests for encoding settings of filestream input (#24426)

* [Elastic Agent] Add the ability to provide custom CA's inside of docker. (#24486)

* Add the ability to provide custom CA's for Elastic Agent docker.

* Add changelog.

* Update Golang to 1.15.9 (#24442)

* Add syntax for multiple selector logging (#24207) (#24497)

* Add syntax for multiple selector logging

* Update libbeat/docs/loggingconfig.asciidoc

Co-authored-by: EamonnTP <[email protected]>

Co-authored-by: EamonnTP <[email protected]>

Co-authored-by: AndyHunt66 <[email protected]>

* chore(ci): use beat_version instead of PR version (#24446)

* Add test for close.reader.after_interval to filestream input (#24423)

* Refactor use of system.hostfs to fix cgroup metrics (#24334)

* refactor use of system.hostfs to fix cgroup metrics

* add changelog

* remove comment

* add cfgwarn

* move changelog

* shift around CLI config location and rep warning

* add comment about system.hostfs usage

* update docs

* capitalization

* fix grammar, add conditional

* change docs phrasing

* [Elastic Agent] Add verification check when updating communication to Kibana. (#24489)

* Add verification check when updating communication to Kibana.

* Add changelog.

* Add const.

* Fix typo in mqtt input docs (#24509)

* Update input-http-endpoint.asciidoc (#24490)

* [Ingest Manager] Move logging defaults to agent (#24535)

[Ingest Manager] Move logging defaults to agent (#24535)

* Clarify that the Tomcat module is for ingesting access logs (#24543)

The Tomcat module is for ingesting access logs, not Catalina or localhost logs.

* [Auditbeat] btmp offset check (#24515)

* auditbeat btmp offset check

Add check that saved offset is not larger than the current file size
to prevent seeking past the end of file

* [Heartbeat] Produce error rather than panic on missing source (#24404)

Fixes #24403.

With the changes to the heartbeat config syntax in 7.12 the `source`
field is now required. Our config validation code didn't actually check
for this field's presence, which caused an NPE.

This PR adds a validation checking for that config's presence. It also
adds tests for the validation code for config sub-fields. There were no
defects found in the validations for source.inline, or source.browser,
but a few tests were missing.

Instead of the panic seen in #24403 users will now get the error seen
below.

```
2021-03-05T15:41:40.146-0600	ERROR	instance/beat.go:952	Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
```

* Fix default scope in add_nomad_metadata (#24559)

Fix default scope in add_nomad_metadata. It is set to local, but
it should be node. Fix also error message that showed that local
is a valid value.

* [Filebeat] Add Dashboards to Threat Intel Module (#24488)

* added dashboards & docs

* ran mage fmt update

* [CI] bump gvm version and use the binary (#24571)

* [CI] Add resilience when installing required tooling (#24542)

* [CI] enable new flaky detector (#24464)

* chore: do not pass beat version (#24586)

We will be delegating the version calculation to the e2e tests, using
target branch values as defaults

Co-authored-by: Jaime Soriano Pastor <[email protected]>
Co-authored-by: Michal Pristas <[email protected]>
Co-authored-by: Noémi Ványi <[email protected]>
Co-authored-by: Blake Rouse <[email protected]>
Co-authored-by: EamonnTP <[email protected]>
Co-authored-by: AndyHunt66 <[email protected]>
Co-authored-by: Manuel de la Peña <[email protected]>
Co-authored-by: Alex K <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: Lee Hinman <[email protected]>
Co-authored-by: Andrew Cholakian <[email protected]>
Co-authored-by: Andrew Pease <[email protected]>
Co-authored-by: Victor Martinez <[email protected]>
(cherry picked from commit 5c6f1b6)
narph added a commit that referenced this pull request Mar 26, 2021
* Update gosigar package after fix (#24502)

* mofidy doc

* work on fix

* update changelog

* update notice

* Update notice

* [Ingest Manager] Sync on rename on windows (#24504)

* Add tests for encoding settings of filestream input (#24426)

* [Elastic Agent] Add the ability to provide custom CA's inside of docker. (#24486)

* Add the ability to provide custom CA's for Elastic Agent docker.

* Add changelog.

* Update Golang to 1.15.9 (#24442)

* Add syntax for multiple selector logging (#24207) (#24497)

* Add syntax for multiple selector logging

* Update libbeat/docs/loggingconfig.asciidoc

Co-authored-by: EamonnTP <[email protected]>

Co-authored-by: EamonnTP <[email protected]>

Co-authored-by: AndyHunt66 <[email protected]>

* chore(ci): use beat_version instead of PR version (#24446)

* Add test for close.reader.after_interval to filestream input (#24423)

* Refactor use of system.hostfs to fix cgroup metrics (#24334)

* refactor use of system.hostfs to fix cgroup metrics

* add changelog

* remove comment

* add cfgwarn

* move changelog

* shift around CLI config location and rep warning

* add comment about system.hostfs usage

* update docs

* capitalization

* fix grammar, add conditional

* change docs phrasing

* [Elastic Agent] Add verification check when updating communication to Kibana. (#24489)

* Add verification check when updating communication to Kibana.

* Add changelog.

* Add const.

* Fix typo in mqtt input docs (#24509)

* Update input-http-endpoint.asciidoc (#24490)

* [Ingest Manager] Move logging defaults to agent (#24535)

[Ingest Manager] Move logging defaults to agent (#24535)

* Clarify that the Tomcat module is for ingesting access logs (#24543)

The Tomcat module is for ingesting access logs, not Catalina or localhost logs.

* [Auditbeat] btmp offset check (#24515)

* auditbeat btmp offset check

Add check that saved offset is not larger than the current file size
to prevent seeking past the end of file

* [Heartbeat] Produce error rather than panic on missing source (#24404)

Fixes #24403.

With the changes to the heartbeat config syntax in 7.12 the `source`
field is now required. Our config validation code didn't actually check
for this field's presence, which caused an NPE.

This PR adds a validation checking for that config's presence. It also
adds tests for the validation code for config sub-fields. There were no
defects found in the validations for source.inline, or source.browser,
but a few tests were missing.

Instead of the panic seen in #24403 users will now get the error seen
below.

```
2021-03-05T15:41:40.146-0600	ERROR	instance/beat.go:952	Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
```

* Fix default scope in add_nomad_metadata (#24559)

Fix default scope in add_nomad_metadata. It is set to local, but
it should be node. Fix also error message that showed that local
is a valid value.

* [Filebeat] Add Dashboards to Threat Intel Module (#24488)

* added dashboards & docs

* ran mage fmt update

* [CI] bump gvm version and use the binary (#24571)

* [CI] Add resilience when installing required tooling (#24542)

* [CI] enable new flaky detector (#24464)

* chore: do not pass beat version (#24586)

We will be delegating the version calculation to the e2e tests, using
target branch values as defaults

Co-authored-by: Jaime Soriano Pastor <[email protected]>
Co-authored-by: Michal Pristas <[email protected]>
Co-authored-by: Noémi Ványi <[email protected]>
Co-authored-by: Blake Rouse <[email protected]>
Co-authored-by: EamonnTP <[email protected]>
Co-authored-by: AndyHunt66 <[email protected]>
Co-authored-by: Manuel de la Peña <[email protected]>
Co-authored-by: Alex K <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: Lee Hinman <[email protected]>
Co-authored-by: Andrew Cholakian <[email protected]>
Co-authored-by: Andrew Pease <[email protected]>
Co-authored-by: Victor Martinez <[email protected]>
(cherry picked from commit 5c6f1b6)

* upadte changelog
@schans
Copy link

schans commented Apr 3, 2021

Hi!

Looks like we missed the 7.12.0 release. Is this still on schedule for 7.13.0? Any plans on backporting to the 7.12 branch?

Thanks
Bart

@fearful-symmetry
Copy link
Contributor Author

Bah, sorry, I expected the PR to get bumped by Steffen, so I missed it.

fearful-symmetry added a commit to fearful-symmetry/beats that referenced this pull request Apr 6, 2021
* refactor use of system.hostfs to fix cgroup metrics

* add changelog

* remove comment

* add cfgwarn

* move changelog

* shift around CLI config location and rep warning

* add comment about system.hostfs usage

* update docs

* capitalization

* fix grammar, add conditional

* change docs phrasing

(cherry picked from commit c70fe5c)
@fearful-symmetry
Copy link
Contributor Author

So, @schans we should be ready for 7.13, however, this is too major to backport to a future 7.12 release.

Comment on lines +119 to +125

Example:

[source,yaml]
------------------------------------------------------------------------------
path.logs: /var/log/beats
------------------------------------------------------------------------------
Copy link
Member

Choose a reason for hiding this comment

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

@fearful-symmetry Is this the correct example? It seems to be a duplicate of the logs example one section up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops!

leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…lastic#24627)

* Update gosigar package after fix (elastic#24502)

* mofidy doc

* work on fix

* update changelog

* update notice

* Update notice

* [Ingest Manager] Sync on rename on windows (elastic#24504)

* Add tests for encoding settings of filestream input (elastic#24426)

* [Elastic Agent] Add the ability to provide custom CA's inside of docker. (elastic#24486)

* Add the ability to provide custom CA's for Elastic Agent docker.

* Add changelog.

* Update Golang to 1.15.9 (elastic#24442)

* Add syntax for multiple selector logging (elastic#24207) (elastic#24497)

* Add syntax for multiple selector logging

* Update libbeat/docs/loggingconfig.asciidoc

Co-authored-by: EamonnTP <[email protected]>

Co-authored-by: EamonnTP <[email protected]>

Co-authored-by: AndyHunt66 <[email protected]>

* chore(ci): use beat_version instead of PR version (elastic#24446)

* Add test for close.reader.after_interval to filestream input (elastic#24423)

* Refactor use of system.hostfs to fix cgroup metrics (elastic#24334)

* refactor use of system.hostfs to fix cgroup metrics

* add changelog

* remove comment

* add cfgwarn

* move changelog

* shift around CLI config location and rep warning

* add comment about system.hostfs usage

* update docs

* capitalization

* fix grammar, add conditional

* change docs phrasing

* [Elastic Agent] Add verification check when updating communication to Kibana. (elastic#24489)

* Add verification check when updating communication to Kibana.

* Add changelog.

* Add const.

* Fix typo in mqtt input docs (elastic#24509)

* Update input-http-endpoint.asciidoc (elastic#24490)

* [Ingest Manager] Move logging defaults to agent (elastic#24535)

[Ingest Manager] Move logging defaults to agent (elastic#24535)

* Clarify that the Tomcat module is for ingesting access logs (elastic#24543)

The Tomcat module is for ingesting access logs, not Catalina or localhost logs.

* [Auditbeat] btmp offset check (elastic#24515)

* auditbeat btmp offset check

Add check that saved offset is not larger than the current file size
to prevent seeking past the end of file

* [Heartbeat] Produce error rather than panic on missing source (elastic#24404)

Fixes elastic#24403.

With the changes to the heartbeat config syntax in 7.12 the `source`
field is now required. Our config validation code didn't actually check
for this field's presence, which caused an NPE.

This PR adds a validation checking for that config's presence. It also
adds tests for the validation code for config sub-fields. There were no
defects found in the validations for source.inline, or source.browser,
but a few tests were missing.

Instead of the panic seen in elastic#24403 users will now get the error seen
below.

```
2021-03-05T15:41:40.146-0600	ERROR	instance/beat.go:952	Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
```

* Fix default scope in add_nomad_metadata (elastic#24559)

Fix default scope in add_nomad_metadata. It is set to local, but
it should be node. Fix also error message that showed that local
is a valid value.

* [Filebeat] Add Dashboards to Threat Intel Module (elastic#24488)

* added dashboards & docs

* ran mage fmt update

* [CI] bump gvm version and use the binary (elastic#24571)

* [CI] Add resilience when installing required tooling (elastic#24542)

* [CI] enable new flaky detector (elastic#24464)

* chore: do not pass beat version (elastic#24586)

We will be delegating the version calculation to the e2e tests, using
target branch values as defaults

Co-authored-by: Jaime Soriano Pastor <[email protected]>
Co-authored-by: Michal Pristas <[email protected]>
Co-authored-by: Noémi Ványi <[email protected]>
Co-authored-by: Blake Rouse <[email protected]>
Co-authored-by: EamonnTP <[email protected]>
Co-authored-by: AndyHunt66 <[email protected]>
Co-authored-by: Manuel de la Peña <[email protected]>
Co-authored-by: Alex K <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: Lee Hinman <[email protected]>
Co-authored-by: Andrew Cholakian <[email protected]>
Co-authored-by: Andrew Pease <[email protected]>
Co-authored-by: Victor Martinez <[email protected]>
(cherry picked from commit b5b66f5)

* upadte changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Team:Integrations Label for the Integrations team v7.11.2 v7.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants