Skip to content

all: limit request tracer log count to five#8489

Merged
efd6 merged 2 commits intoelastic:mainfrom
efd6:maxlogs_tracers
Nov 15, 2023
Merged

all: limit request tracer log count to five#8489
efd6 merged 2 commits intoelastic:mainfrom
efd6:maxlogs_tracers

Conversation

@efd6
Copy link
Copy Markdown
Contributor

@efd6 efd6 commented Nov 14, 2023 •
edited
Loading

Proposed commit message

Currently the request tracer log is configured to retain all logs. This opens the risk of user filesystems being filled with unwanted logs. So limit the number of logs to five (picked as reasonably similar to the number of logs retained by elastic-agent, although the logs are not necessarily comparable in size or temporal domain).

Relevant docs for the change:

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

@efd6 efd6 added enhancement New feature or request Team:Security-External Integrations Team:Elastic-Agent-Data-Plane Agent Data Plane team [elastic/elastic-agent-data-plane] Team:Cloud-Monitoring Label for the Cloud Monitoring team Team:Obs-InfraObs Observability Infrastructure Monitoring team [elastic/obs-infraobs-integrations] labels Nov 14, 2023
@efd6 efd6 self-assigned this Nov 14, 2023
@efd6 efd6 force-pushed the maxlogs_tracers branch 3 times, most recently from d3d7d1d to 80ae524 Compare November 14, 2023 01:28
@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Nov 14, 2023 •
edited
Loading

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2023-11-15T21:10:25.772+0000

  • Duration: 94 min 47 sec

Test stats 🧪

Test Results
Failed 0
Passed 2251
Skipped 3
Total 2254

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Nov 14, 2023 •
edited
Loading

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (272/272) 💚
Files 95.965% (333/347) ❕
Classes 95.965% (333/347) ❕
Methods 93.648% (3671/3920) ❕
Lines 91.409% (93762/102574) ❕
Conditionals 100.0% (0/0) 💚

@efd6
Copy link
Copy Markdown
Contributor Author

efd6 commented Nov 14, 2023

/test

2 similar comments
@efd6
Copy link
Copy Markdown
Contributor Author

efd6 commented Nov 14, 2023

/test

@efd6
Copy link
Copy Markdown
Contributor Author

efd6 commented Nov 14, 2023

/test

@efd6 efd6 marked this pull request as ready for review November 14, 2023 20:59
@efd6 efd6 requested review from a team as code owners November 14, 2023 20:59
@efd6 efd6 requested review from belimawr and rdner November 14, 2023 20:59
@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Nov 14, 2023

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Nov 14, 2023

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@ebeahan
Copy link
Copy Markdown
Member

ebeahan commented Nov 14, 2023 •
edited
Loading

So the configuration would now limit to 5 request tracer logs, each 1MB in size per the resource.tracer.maxsize default?

@efd6
Copy link
Copy Markdown
Contributor Author

efd6 commented Nov 14, 2023

Yes, that's approximately correct. There are a couple of integrations that limit log size to 5MB in their configs here, but all are now limited to 5 files.

ebeahan
ebeahan approved these changes Nov 14, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@ebeahan ebeahan left a comment

Choose a reason for hiding this comment

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

++

Initially, I worried slightly about limiting to 5MB being too limited for certain troubleshooting situations. However, I think it's a sensible starting point and better than inadvertently filling disk if the toggle is turned on and forgotten. 😅

@efd6
Copy link
Copy Markdown
Contributor Author

efd6 commented Nov 15, 2023

/test

leehinman
leehinman approved these changes Nov 15, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@leehinman leehinman left a comment

Choose a reason for hiding this comment

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

Changes to httpjson.yml.hbs look good, but I did notice 2 changelog.yml changes that looked odd.

packages/hashicorp_vault/changelog.yml Outdated
Copy link
Copy Markdown
Contributor

@leehinman leehinman Nov 15, 2023

Choose a reason for hiding this comment

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

Is this right PR number for this change?

Copy link
Copy Markdown
Contributor Author

@efd6 efd6 Nov 15, 2023

Choose a reason for hiding this comment

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

No, no it is not. I will repair that to what it should have been.

packages/o365/changelog.yml Outdated
Copy link
Copy Markdown
Contributor

@leehinman leehinman Nov 15, 2023

Choose a reason for hiding this comment

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

Is this the right PR number for this change?

Copy link
Copy Markdown
Contributor Author

@efd6 efd6 Nov 15, 2023

Choose a reason for hiding this comment

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

Likewise.

muthu-mps
muthu-mps reviewed Nov 15, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@muthu-mps muthu-mps left a comment

Choose a reason for hiding this comment

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

If we are setting a default limit for the request tracer maxbackups. Can we update the documentation to have a mention about the default limit here?
Otherwise change looks good!

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Nov 16, 2023

Package tines - 1.9.0 containing this change is available at https://epr.elastic.co/search?package=tines

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Nov 16, 2023

Package trellix_epo_cloud - 1.9.0 containing this change is available at https://epr.elastic.co/search?package=trellix_epo_cloud

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Nov 16, 2023

Package trend_micro_vision_one - 1.15.0 containing this change is available at https://epr.elastic.co/search?package=trend_micro_vision_one

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Nov 16, 2023

Package windows - 1.43.0 containing this change is available at https://epr.elastic.co/search?package=windows

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Nov 16, 2023

Package wiz - 0.4.0 containing this change is available at https://epr.elastic.co/search?package=wiz

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Nov 16, 2023

Package zeek - 2.22.0 containing this change is available at https://epr.elastic.co/search?package=zeek

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Nov 16, 2023

Package zerofox - 1.22.0 containing this change is available at https://epr.elastic.co/search?package=zerofox

@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Nov 16, 2023

Package zeronetworks - 1.11.0 containing this change is available at https://epr.elastic.co/search?package=zeronetworks

@andrewkroh andrewkroh added Integration:akamai Akamai (Community supported) Integration:atlassian_bitbucket Atlassian Bitbucket (Community supported) Integration:atlassian_jira Atlassian Jira (Community supported) Integration:box_events Box Events Integration:cisco_duo Cisco Duo Integration:cloudflare Cloudflare (Community supported) Integration:apache Apache HTTP Server Integration:aws AWS Integration:citrix_adc Citrix ADC Integration:github GitHub Integration:atlassian_confluence Atlassian Confluence (Community supported) Integration:couchbase Couchbase Integration:forgerock ForgeRock Integration:cisco_secure_endpoint Cisco Secure Endpoint Integration:darktrace Darktrace Integration:1password 1Password (Partner supported) Integration:bitdefender BitDefender (Community supported) Integration:bitwarden Bitwarden Integration:carbon_black_cloud VMware Carbon Black Cloud Integration:cel Custom API using Common Expression Language Integration:ceph Ceph labels Jul 22, 2024
@efd6 efd6 deleted the maxlogs_tracers branch February 5, 2025 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zmoog zmoog zmoog approved these changes

@ebeahan ebeahan ebeahan approved these changes

@leehinman leehinman leehinman approved these changes

@muthu-mps muthu-mps muthu-mps approved these changes

@belimawr belimawr Awaiting requested review from belimawr belimawr is a code owner automatically assigned from elastic/elastic-agent-data-plane

@rdner rdner Awaiting requested review from rdner rdner is a code owner automatically assigned from elastic/elastic-agent-data-plane

Assignees

@efd6 efd6

Labels

enhancement New feature or request Integration:akamai Akamai (Community supported) Integration:apache Apache HTTP Server Integration:atlassian_bitbucket Atlassian Bitbucket (Community supported) Integration:atlassian_confluence Atlassian Confluence (Community supported) Integration:atlassian_jira Atlassian Jira (Community supported) Integration:aws AWS Integration:bitdefender BitDefender (Community supported) Integration:bitwarden Bitwarden Integration:box_events Box Events Integration:carbon_black_cloud VMware Carbon Black Cloud Integration:cel Custom API using Common Expression Language Integration:ceph Ceph Integration:cisco_duo Cisco Duo Integration:cisco_secure_endpoint Cisco Secure Endpoint Integration:citrix_adc Citrix ADC Integration:cloudflare Cloudflare (Community supported) Integration:couchbase Couchbase Integration:darktrace Darktrace Integration:forgerock ForgeRock Integration:github GitHub Integration:1password 1Password (Partner supported) Team:Cloud-Monitoring Label for the Cloud Monitoring team Team:Elastic-Agent-Data-Plane Agent Data Plane team [elastic/elastic-agent-data-plane] Team:Obs-InfraObs Observability Infrastructure Monitoring team [elastic/obs-infraobs-integrations]

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@efd6 @elasticmachine @ebeahan @zmoog @leehinman @muthu-mps @andrewkroh