Skip to content

Add null logging to avoid issues on load tests#14754

Closed
lucasmrod wants to merge 1 commit intomainfrom
null-logging-for-loadtest
Closed

Add null logging to avoid issues on load tests#14754
lucasmrod wants to merge 1 commit intomainfrom
null-logging-for-loadtest

Conversation

@lucasmrod
Copy link
Copy Markdown
Member

@lucasmrod lucasmrod commented Oct 26, 2023

We really needed something like this during load tests for #7766 (to not get those Firehose errors while working at a high load using scheduled queries). This will be useful for any subsequent load test.

@rachaelshaw/@noahtalerman Let me know if:

  • We want to name it something different than null.
  • We want to hide this option from our docs. (*)
  • We want a different wording on the UI when using this. (*)

Let me know if you want me to create an issue for this.

(*) AFAICS this is only useful for load-testing/internal-use.

Screenshot 2023-10-26 at 14 45 00

@lucasmrod lucasmrod requested review from a team and rachaelshaw as code owners October 26, 2023 17:44
@lucasmrod lucasmrod requested review from mostlikelee and removed request for mikermcneil October 26, 2023 17:44
@lucasmrod lucasmrod temporarily deployed to Docker Hub October 26, 2023 17:44 — with GitHub Actions Inactive
@lucasmrod lucasmrod requested a review from jahzielv October 26, 2023 17:44
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 26, 2023

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (0b650de) 58.85% compared to head (cddfa6e) 58.83%.
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14754      +/-   ##
==========================================
- Coverage   58.85%   58.83%   -0.02%     
==========================================
  Files         925      926       +1     
  Lines       77287    77295       +8     
  Branches     2222     2222              
==========================================
- Hits        45487    45477      -10     
- Misses      28190    28205      +15     
- Partials     3610     3613       +3     
Flag Coverage Δ
backend 59.53% <0.00%> (-0.03%) ⬇️
frontend 51.87% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
server/logging/logging.go 0.00% <0.00%> (ø)
server/logging/null.go 0.00% <0.00%> (ø)
server/service/service_appconfig.go 82.84% <0.00%> (-1.66%) ⬇️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mikermcneil mikermcneil removed their request for review October 27, 2023 17:58
@noahtalerman
Copy link
Copy Markdown
Member

We really needed something like this during load tests for #7766 (to not get those Firehose errors while working at a high load using scheduled queries)

@lucasmrod can't we just use stdout to not get Firehose errors?

@lucasmrod
Copy link
Copy Markdown
Member Author

We really needed something like this during load tests for #7766 (to not get those Firehose errors while working at a high load using scheduled queries)

@lucasmrod can't we just use stdout to not get Firehose errors?

That is possible yes, but may come with a cost ($ for log storage).
Or some infrastructure changes (@rfairburn) to redirect Fleet's stdout to nowhere instead of AWS.

@lucasmrod
Copy link
Copy Markdown
Member Author

We really needed something like this during load tests for #7766 (to not get those Firehose errors while working at a high load using scheduled queries)

@lucasmrod can't we just use stdout to not get Firehose errors?

That is possible yes, but may come with a cost ($ for log storage). Or some infrastructure changes (@rfairburn) to redirect Fleet's stdout to nowhere instead of AWS.

Let us know if you would like to tackle this on the infrastructure side. Such change (or this one) will be necessary to perform new load tests on query reports.

@lucasmrod
Copy link
Copy Markdown
Member Author

@noahtalerman (forgot to cc)

@rfairburn
Copy link
Copy Markdown
Contributor

If you want to really not send the data anywhere, picking a filesystem destination and choosing /dev/null as the file path might be the easiest solution.

@lucasmrod
Copy link
Copy Markdown
Member Author

If you want to really not send the data anywhere, picking a filesystem destination and choosing /dev/null as the file path might be the easiest solution.

Ah good idea. I'll close this and open a separate PR for infrastructure changes.

@lucasmrod
Copy link
Copy Markdown
Member Author

Closed in favor of #14848

lucasmrod added a commit that referenced this pull request Nov 1, 2023
…14848)

This is to avoid firehose (rate limit) errors when load testing query
reports with thousands of hosts. (And may reduce cost by storing logs
nowhere.)

Thanks @rfairburn:
#14754 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants