-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add explicit tracking and ignoring of metrics and sub-metrics #1321
Comments
I think the
In short, use untrack to remove defaults from view, and track to add more things needing to be tracked. Reading the original question on the community board, would a trend custom metric work here? That |
Yeah, sub-metrics would require some more thought... For example, if you untrack the "parent" metric, but explicitly track a few submetrics, should that work? My instinctive answer is "no", but I can see pros and cons for both approaches, so we need to consider that a bit more carefully. And maybe we should have separate functions that deal with submetrics? This would also make implementing the rule matching enhancements from #1313 easier... Or, better yet, expose a proper object with methods to query, track and untrack things?
This makes sense, though it would probably greatly complicate the implementation, both in k6 and in the cloud. We'd need some mechanism to communicate these changes to the cloud, some sort of a meta-metric perhaps? And if we implement the metric tracking/untracking as a local per-VU thing, so that you can untrack some metrics midway through the VU execution, it would be difficult to square that with what we display in the test run view...
You're right, this is a valid workaround, I posted it in the forum. Not sure what you mean by "That emits at the end of the test." though 😕 |
I don't think the "flexible" approach is actually what we want(or at least I see no real benefits to it):
res = http.get("something");
If (rest.status == 404) {
trackMetric('http_req_duration{"url": "'+res.url+'"}');
} I mean I guess it is powerful but do you think that will work - it won't (or it is going to miss data) and if you can't do that having it as an option localizes it one place instead of giving it the ability to be hard to find in big code bases.
IMO this being another option(s) have a lot more benefits:
import { trackMetric } from "k6/metrics";
// metric not defined up to here
trackMetric("custom_metric{tag:something}"); Should All of those are no problem in a single file 50-60 lines codebases, but we do have a lot of people with arguably whole frameworks around k6 in which this will add ... IMO unneeded complexity. {
"tracked_metrics": ["metric1","metric2"]
} mean track only metrics: "metric1", "metric2" and nothing else with a good default value: {
"additionally_track_metrics": ["metric3","metric4"]
} which means to the metrics that already are tracked ... track those as well. This will return an error if combined with the above "tracked_metrics", {
"do_not_track_metrics": ["metric5"]
} can be combined with Implementation algorithm:
I agree that this adds complexity to the configuration .. but not really any more then it already is there and I think this complexity is a lot better than adding 1 key and then 2 functions that we need to take care of on top of. This also can be easily expected in the cloud output at least as all the options are sent either way. I also think that this should be considered with the future metric aggregation in mind. Also, I would like to point out that the cloud output might need 1 or more of the metrics either way in order to function properly and I am against the user being able to break the cloud output when they use it and as such we probably will need a mechanism to tell them that they can't disable "http_req_duration" for example. Also, this probably should prevent people from disabling metrics that are used in thresholds? |
I agree with the drawbacks of proposal #1, those seem like deal breakers. The second functional approach is interesting, but it will only complicate scripts and our implementation if we decide to support AFAICS, this is a global runtime setting, and as such it makes more sense to me for it to be configured via a CLI flag / env var. For this to work simply, we probably need to structure the metric names better and for the config option to support wildcards or regex. Consider:
I think this makes intuitive sense as to what is being tracked. Note that we could structure child metrics with I don't think adding the functionality of untracking metrics mid-test would be worth the added complexity. |
Hmm seems like I didn't explain my "localized tracking and untracking" idea properly... The above example doesn't make sense, because untrackMetric("http_req_duration");
// A region of code that makes a bunch of requests you don't want
// to track. There are many reasons users may want that - some
// setup() code that's not important, or requests that they know
// are slow and don't want to pollute the rest of the metrics, or
// expected errors, etc.
http.get("https://myapi.com/some-super-slow-call-I-don't-care-about")
// ...
trackMetric("http_req_duration");
As I mentioned above, we probably should have separate methods for submetrics, so we don't rely quite so much on arcane syntax. In any case, I think we should just do the normal thing in such a situation and throw an exception 😉.
I'm not sure aborting the execution makes sense - after all, a metric can receive data points even at the very end of the execution, in the And while in a lot of cases it would be a good UX to emit a warning if a metric was (explicitly or implicitly) tracked, but didn't get any data, there are cases where it doesn't make sense... For example, say that you have an error So, I think the reasonable approach here is that, if the user implicitly or explicitly tracked a metric, we should show it in the end-of-test summary, even if it's to show
I think yes... eventually. It would be better UX, but it seems like we can implement it as a separate PR, after we implement the rest. That's what I meant by "This makes sense, though it would probably greatly complicate the implementation, both in k6 and in the cloud." above...
If you mean "configure an array and reuse that", this has all of the drawbacks I mentioned above under "but it has some big drawbacks"... Even if we use a proper option to configure this, I'd still like to heavily discourage this approach. And if you mean import { trackMetrics } from "k6/metrics";
import { myImportantMetricsArray } from "./some-common-file.js"
trackMetrics(...myImportantMetricsArray); The only difference is that with the local per-VU approach, you have the added flexibilty.
I don't understand what you mean here, sorry.
I very much dislike this approach... 😅 To me it seems to be more complicated for users, more complicated in terms of #883, and less flexible than either of the approaches that used functions. And I don't exaggerate the user-facing complexity, it literally took me 3 readings to understand what you're proposing, and I'm still probably misunderstanding something. For example:
Wat, why?
Agree, though I don't currently see a lot of overlap between these things. That is, I don't see how any metric whitelist/blacklist option, or even its lack, would affect the aggregation. The most relevant thing I can think of is the aforementioned meta-metric that could be used to notify outputs of the existence of metrics/sub-metrics. This probably could be reused for aggregation "announcements"/options...
Please refer to the "Outputs should probably also be able to specify certain metrics that they need" part from my initial proposal..
Ditto, "Each threshold should also implicitly call trackMetric()..." in the original post/
Whether we're using a global option or a local per-VU metric tracking, the complexity, performance, and logic of the code wouldn't be substantially changed. Only, in one case you'd execute that logic somewhat centrally, and in the other case you'd do it independently in each VU. Thinking about it some more, the local option might even be a bit more performant... Also, we're not doing something wildly dissimilar even now. Refer to the
I'm not against having a global CLI flag / env var., but this has the same drawbacks of my first global
I'd have very much liked if we could do something like this... 😞 Unfortunately, because metrics in k6 scripts can originate organically in the middle of the script, we can't know all of their names in advance. "Knowing the metric names in advance" is half of the idea for having an explicit whitelist... So, since we can't know all possible metric names before the script starts, this approach would be very costly in terms of CPU usage, because we'd have to save all of these wildcard patterns without resolving them. Then, whenever a metric is emitted, we'd have to validate its name against them. Even if we used tries, it seems to me like this would be too CPU heavy when you have to do it potentially a several million times per test run 😞 The alternative of having a simple whitelist/blacklist would just be a fast lookup in a small map.
I'm more and more of the opposite opinion... 😅 The thing is that the more I consider the alternatives, the more appealing the local per-VU metric tracking and untracking becomes... Or, at the most, the hybrid approach. My reasons are twofold:
|
Hey. I was the one who asked the question over on Community. Thanks for raising this and driving it forward. To explain a little more where this is coming from the problem we were trying to solve was to succinctly show more detail about different endpoints being called in the same tests so we can see which ones are slower, etc... The tag + threshold functionality does this nicely for us without having to add in a lot of custom metrics for each endpoint (and then multiple thereof for each data point we wanted to track).
Also agree with this except that we're also using http_req_waiting to monitor time to first byte so it would be good to have this kept or a TTFB metric added instead. Many thanks again for driving this. Really happy with k6 and excited to see where it goes next. |
Thanks for chiming in!
Do you think the approach of enumerating these endpoints and calling something like
TTFB definitely seems like a more useful metric than most of the ones we show right now... We currently measure and display the fundamental parts of the HTTP request making process, The only composite metric we currently show is That said, it's obvious that network delays shouldn't be ignored. So I'd be perfectly fine with adding a new composite metric we show by default called But I think we, by default, should display the "minor" constituent metrics only if users explicitly request it, or have thresholds based on them. I think that, in most use cases, you'd only want to look at these if you're debugging an issue? And it'd be much easier to initially spot an issue with the composite metrics, since they can surface them much more easily and visibly, I think... And if |
We can very easily add such a helper function as a simple and fool-proof way to track/untrack metrics per-VU. This way there wouldn't be any issues with surviving exceptions and code bugs, would be to have a helper API like this: withUntrackedMetrics(["http_req_duration{status:403}", "http_req_ttfb"], function() {
// some user code that doesn't want to measure the metrics above,
// but may throw an exception without messing the state
}) Basically, this is like |
Hmm that seems fine to me at first glance yeah.
Yeah I can agree on this. What I meaning though was you remove the minor metrics and leave |
I am also very interested in this feature primarily because the disk usage of our InfluxDB increases > 1 GB/hour when our test is running. Unless we pay extra attention, the disk usage could reach to the limit (we set it to 100 GB) any time, like at midnight or on weekends. I hope this feature will be implemented in the near future. Thanks. While waiting, I am thinking of making a workaround for the InfluxDB disk usage issue, i.e. just suppress writing to InfluxDB for some metrics specified through an environment variable. When I could build such a workaround, I will push the code to my forked k6 repository. |
I could build a workaround described above: TamiTakamiya@566b0e6 This simply suppress saving metrics specified in the environment variable |
I described a very partial workaround here, for anyone that doesn't want their |
Another use case for giving users the ability to say "keep track of all submetrics based on some tags": https://community.k6.io/t/how-to-get-failure-statistics/1247 If users could specify that k6 should "keep track of all sets of |
I came to Exclude Given the k6 built-in metrics, the selected HTTP request/s should not affect the general I read a few proposals. Subscribing to the issue to be informed about the final API proposal. |
Subscribed to the issue. |
No firm plans yet, sorry 😞 Though the metric refactoring we did in the recently released k6 v0.41.0 should hopefully make implementing this feature significantly easier 🤞 I think the biggest blocker right now is the fact that we don't have a good proposal for how the JS API should look like 🤔 |
Subscribed to the issue. Any plans for this? 🤞 |
Hi @sebastianpujina, this feature is one of our high priorities in our backlog for the Metrics area. In the next cycle, we could consider starting to work on a JavaScript API for it. Remember to upvote the issue using the thumbs-up reaction so it will be balanced appropriately during the evaluation phase. Currently, there isn't any update to the @na--'s #1321 (comment). |
Having the ability to disable or discard specific metrics collected within a block directly from the JavaScript API would be incredibly useful. Are there any plans to implement this feature, or something along those lines? One practical example where this would be beneficial is implementing a function that retries a request until it succeeds. This would allow us to avoid hardcoding the status code in threshold names e.g., |
k6 measures a lot of things, and it can and can generate quite a lot of data points for a variety of different metrics. Not everything we measure is important to everyone, and we want to measure even more things - #877, #880, #888, #921, #1311, and all of the non-HTTP protocols we want to add...
Unfortunately, currently there's no way for a user to tell k6 that they're not interested in some of the things it measures (#570). That's a problem, because measuring and reporting useless things adds a significant overhead both to k6 (#764), and to any custom outputs that users pipe the metrics data to (#1060).
On top of that, some of the current default metrics just add clutter and confusion to the end-of-test summary report (#1319), without a lot of benefit to most users. All of the
http_req_*
"minor" metrics probably shouldn't be enabled by default, instead we should probably have onlyhttp_req_duration
, which issending + waiting + receiving
. Maybe we should also expose the composite metric for the connection establishment, but I doubt most people would care abouthttp_req_tls_handshaking
individually, most of the time.group_duration
is also somewhat useless when you have a variety of different groups and don't use an external metrics output... Unfortunately, we currently can't remove any of these metrics by default, since we don't have a mechanism for users to enable them if they want/need them...There's also no way besides thresholds for users to force k6 to calculate and display information about sub-metrics, i.e. specific subsets of the data points of the main metrics. This point was recently raised by a user in the community forum, and unfortunately, thresholds is the only way to tell k6 to keep track of sub-metrics right now 😞
So, I think I've listed all of the connected issues I can remember... My proposed solution, as suggested by the issue title, is to allow users to whitelist metrics and sub-metrics. k6 should have a default whitelist, but we should allow users to set their own. Here is a suggestion for the simplest possible approach we can offer, though I think this might not be the best solution:
This would work... somewhat..., but it has some big drawbacks:
http_req_errors
(Proposal for addinghttp_req_errors
metric #1311), most users that had already configuredmetrics
themselves won't be aware of them, since they would be excluded, even if we show them by default in new k6 versions - that would be overwritten...metrics
option, our default metrics handling somehow has to make an exception for them and track and show the custom metrics' values anyway... After all, if a user went to the trouble of measuring something manually with a custom metrics, they probably care about it and want to see it in the results...That said, I'm not firmly against still have such an option - it allows users explicitly and exactly to specify the metrics they want. But, if we do have it, I'd like us to heavily discourage its usage, and offer a better alternative for 99% of use cases.
I'd argue that the vast majority of users and use cases would be satisfied if we just had a simple way for adding metrics to the list of default metrics. So, I think we should add a simple new
trackMetrics()
function (I'd appreciate if someone can think of a better name..) to thek6/metrics
package, which can be used like this:Some details:
metrics
that we'd ship with k6, but will non-destructively add to themapply()
, you can easily pass an array as well.untrackMetric(name)
function, just so that users aren't forced to use themetrics
option directly.trackMetric()
. This way, all custom metrics would be measured and shown in the test results. Not sure if we need to add a parameter to these constructors to disable this behavior, or if users can just calluntrackMetric()
.trackMetric()
for the metric or sub-metric it was based on.group_duration
metric doesn't make a lot of sense if you just run k6 locally, but it's very useful for thecloud
output. That said, this addition feels optional, not a requirement, so I think it should be overrideable byuntrackMetric("group_duration")
.Implementation-wise, all of these function calls would probably matter only in the goja runtime that's executing the init context to get the exported options, but I'm not sure this is something worth optimizing.
Alternatively, we may be able to go off the trodden path and side-step the current k6 configuration entirely. I'm not sure if this would make the configuration mess (#883) worse, or if it won't affect it at all, but I'm hopeful for the second... In any case, we can probably mostly handle the metrics whitelist as a localized per-VU thing that doesn't touch the global configuration. So, no global
metrics
key in the exportedoptions
, just some local per-VU logic that decides which metrics it's going to emit.The biggest benefit of the alternative approach I can think of is that we'd get a ton of flexibility. Different VUs will be able to emit different metrics. Even better, a VU could temporarily stop emitting certain metrics, and later resume that emission, which has been requested before: #1298, #884 (comment) , #891 (comment). The biggest issues with this approach I can think of are:
metrics
option, but VUs can control metric emission locally. 🤷♂️Open questions:
The text was updated successfully, but these errors were encountered: