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

Support thresholds and the end-of-test summary in distributed execution #3213

Open
wants to merge 1 commit into
base: basic-distributed-execution
Choose a base branch
from

Conversation

na--
Copy link
Member

@na-- na-- commented Jul 19, 2023

What?

This PR is another sliver of the newly refactored #2816. It builds on top of #3205 to add very basic support for metrics (thresholds and the end-of-test summary, incl. handleSummary()) for distributed execution.

As you can see from all of the TODOs and the lack of tests, this is even more of a proof-of-concept PR than even #3205 😅 It is very far from ready for any kind of actual production usage!

Even if we ignore the super-inefficient string-y and JSON encoding of metrics data and their transmission over the wire, k6 still doesn't have HDR histograms (#763). So the central k6 coordinator node will suffer from the same memory and data processing problems k6 currently does, it will require tons of memory and CPU for crunching the metrics data of large or long-running tests...

Why?

Same as the other PRs in this series, I decided to create this PR even when the feature isn't production-ready because I think merging it this way will make it easier to build the final end result in smaller and faster iterations.

Once again, this PR is completely backwards compatible! 🎉 It doesn't have any tests, but it also doesn't disable or change any existing tests or k6 behavior. It only affects distributed execution which, if #3205 is merged, would be kind of hidden and officially quite experimental. But it mostly works and it doesn't need HDR histograms to be figured out before it can be merged! 🎉

So it makes sense to me to have this basic framework merged first and then work on HDR histograms (and the other features that would make distributed execution production-ready) in separate PRs.

Checklist

  • I have performed a self-review of my code.
  • I have added tests for my changes.
  • I have run linter locally (make ci-like-lint) and all checks pass.
  • I have run tests locally (make tests) and all tests pass.
  • I have commented on my code, particularly in hard-to-understand areas.

Related PR(s)/Issue(s)

@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (basic-distributed-execution@2443ac6). Click here to learn what that means.

❗ Current head ff58304 differs from pull request most recent head 3363208. Consider uploading reports for the commit 3363208 to get more accurate results

Additional details and impacted files
@@                      Coverage Diff                       @@
##             basic-distributed-execution    #3213   +/-   ##
==============================================================
  Coverage                               ?   70.47%           
==============================================================
  Files                                  ?      275           
  Lines                                  ?    20975           
  Branches                               ?        0           
==============================================================
  Hits                                   ?    14782           
  Misses                                 ?     5266           
  Partials                               ?      927           
Flag Coverage Δ
ubuntu 70.41% <0.00%> (?)
windows 70.33% <0.00%> (?)

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

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

@na-- na-- force-pushed the metrics-in-distributed-execution branch from ee06484 to 3363208 Compare December 13, 2023 12:42
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.

4 participants