Skip to content

Add threshold for statistics#465

Draft
pavlogrushetsky wants to merge 10 commits into
PragmaticFlow:devfrom
pavlogrushetsky:dev
Draft

Add threshold for statistics#465
pavlogrushetsky wants to merge 10 commits into
PragmaticFlow:devfrom
pavlogrushetsky:dev

Conversation

@pavlogrushetsky
Copy link
Copy Markdown

@pavlogrushetsky pavlogrushetsky commented May 15, 2022

This is a draft PR for #355 (Add threshold for statistics). The work is still in progress, and the PR is rather for review and conversation about the overall approach.

Some other things TODO:

  • Implementing thresholds for DataTransferStats
  • Implementing logic for displaying the thresholds status in the load test outputs
  • Specifying thresholds through the configuration files
  • Adding comments to the new methods in the contracts
  • Covering the changes with the tests

I would appreciate any feedback on this draft implementation 🙇‍♂️.

Comment thread src/NBomber/NBomber.fsproj
Comment thread src/NBomber/Api/CSharp.fs
Comment thread examples/FSharpDev/HttpTests/SimpleHttpTest.fs Outdated
Comment thread src/NBomber.Contracts/Metrics.fs Outdated
Comment thread src/NBomber.Contracts/Stats.fs Outdated
Comment thread examples/CSharpDev/HttpTests/SimpleHttpTest.cs
Comment thread src/NBomber/Domain/Scenario.fs Outdated
Comment on lines +73 to +81
let private printThresholdStats (scnStats: ScenarioStats) =
let headers = ["threshold"; "status"]
let rows =
scnStats.ThresholdStats
|> Option.map (ReportHelper.ThresholdStats.createTableRows Console.okEscColor Console.errorEscColor)
|> Option.defaultValue List.empty

[ Console.addLine $"thresholds for scenario: {Console.okColor scnStats.ScenarioName}"
Console.addTable headers rows ]
Copy link
Copy Markdown
Author

@pavlogrushetsky pavlogrushetsky May 20, 2022

Choose a reason for hiding this comment

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

That's how it looks like:

image

I'm not sure if this would be sufficient, and if not, I'm not sure at the moment how we could print a string representation for the thresholds predicates.

Copy link
Copy Markdown
Contributor

@AntyaDev AntyaDev May 23, 2022

Choose a reason for hiding this comment

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

For F# we could use https://docs.microsoft.com/en-us/dotnet/fsharp/language-reference/code-quotations
But on the other hand, it will be a deal-breaker for C#.
I am not sure but maybe this project can help here https://mbraceproject.github.io/FsPickler/
I don't exactly know if it is possible to somehow fetch the AST representation of the Threshold function/delegate but it would be awesome.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, we could have an additional API that will accept the optional string argument that the user will fill, and it will be printed if it's not empty. But maybe, for now, we can leave all this "Fetching Meta Info" machinery as for v2?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @pavlogrushetsky
You can take a look how this library is working for F#
https://github.com/SwensenSoftware/unquote

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you! I will take a look.

Copy link
Copy Markdown
Author

@pavlogrushetsky pavlogrushetsky May 24, 2022

Choose a reason for hiding this comment

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

@AntyaDev It looks like I could utilize Unquote library 😀. I've played with it a little bit, and was able to specify a threshold using quotation expression. The work is still in progress, but I think, I would need to separate Threshold model to have one in Contracts with function union cases (like int -> bool), and one in DomainTypes with Quotations.Expr union cases (like Quotations.Expr<int -> bool>). This would allow us to keep the consumer code as simple as possible, to not to have something like the following:

|> Scenario.withThresholds [
    RequestAllCount(<@ fun x -> x = 3 @>)
]

and instead be able to specify thresholds using plain functions:

|> Scenario.withThresholds [
    RequestAllCount(fun x -> x = 3)
]

I would just need to translate Contracts.Threshold model to DomainTypes.Threshold model, like you do for Scenario.

@AntyaDev Please, let me know if this makes sense to you.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, it sounds reasonable!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hey @AntyaDev 👋 I've played a bit with quotation expressions, and it turned out that the text representation for the decompiled predicates isn't really human-readable. I think, it's not worth implementing something very complicated, at least at this point.

So, I've added an ability to override a default threshold description. Here is how it looks in C#:

image

In F#, it looks a little bit ugly:

image

To improve this syntax for F# users, I've implemented a simple computation expression, so that it would be possible to specify a description as a real optional parameter, like the following:

image

I've also improved the console output to make it more human readable. Here is an example:

image

@AntyaDev Please, let me know what you think about this 😅

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks very good, I like how F# and C# look like.
Btw what was the issue with F# quotations? I thought that you would not be able to use it for C# and to use it for F#, you should wrap the predicate into <@ predicate @>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Btw what was the issue with F# quotations? I thought that you would not be able to use it for C# and to use it for F#, you should wrap the predicate into <@ predicate @>

Yep, I was able to wrap the predicates in <@ @> in the domain logic and keep using just plain lambdas like fun x -> x > 10 in both C# and F#, and the thresholds evaluation logic worked good. And main reason to do this was to get a string representation for those quotation expressions, but it turned out that it is not really human readable when I tried to de-compile them.

Comment thread src/NBomber/Domain/Errors.fs Outdated
Comment thread src/NBomber/Domain/Errors.fs
@pavlogrushetsky
Copy link
Copy Markdown
Author

pavlogrushetsky commented May 20, 2022

@AntyaDev One more question 😅 Would the following functionality make sense to you?:

  • Specifying thresholds through the configuration files

@AntyaDev
Copy link
Copy Markdown
Contributor

"Specifying thresholds through the configuration files" - seems can be skipped for now :)
I used to add such things iteratively.

@pavlogrushetsky
Copy link
Copy Markdown
Author

"Specifying thresholds through the configuration files" - seems can be skipped for now :) I used to add such things iteratively.

Great! Makes sense to me, thank you!

@AntyaDev
Copy link
Copy Markdown
Contributor

AntyaDev commented Jul 4, 2022

Hi @pavlogrushetsky
Any news?

@AntyaDev AntyaDev force-pushed the dev branch 3 times, most recently from 60b2d02 to 16d6057 Compare February 3, 2023 10:34
@AntyaDev AntyaDev force-pushed the dev branch 6 times, most recently from cee3bec to 7295146 Compare March 30, 2023 12:56
@AntyaDev AntyaDev added the 4.3 label Apr 11, 2023
@AntyaDev AntyaDev force-pushed the dev branch 2 times, most recently from 4ca4c86 to fc702e8 Compare August 15, 2023 10:26
@AntyaDev AntyaDev removed the 4.3 label Jul 8, 2024
@AntyaDev AntyaDev force-pushed the dev branch 5 times, most recently from cdedaac to c6d5c6e Compare January 7, 2025 08:18
@AntyaDev AntyaDev force-pushed the dev branch 2 times, most recently from 2efc8d7 to 377a3c9 Compare April 22, 2025 18:58
@AntyaDev AntyaDev force-pushed the dev branch 2 times, most recently from 5a9d254 to 1626e74 Compare August 16, 2025 12:57
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.

2 participants