Skip to content

feat: Multiple Executions for PsPingPlugin #434

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.

Sign up for GitHub

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

Open
deyanp wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

feat: Multiple Executions for PsPingPlugin #434

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a8fa065
feat: Multiple Executions for PsPingPlugin
deyanp Nov 18, 2021
c359e5f
chore: Change parallel to sequential execution of the pings with slee…
deyanp Dec 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 70 additions & 34 deletions src/NBomber/Plugins/PsPingPlugin.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,40 @@ open FSharp.Control.Tasks.NonAffine
open FsToolkit.ErrorHandling
open Microsoft.Extensions.Configuration

open NBomber
open NBomber.Contracts
open NBomber.Domain.Stats.Statistics
open NBomber.Extensions.InternalExtensions

[<CLIMutable>]
type PsPingPluginConfig = {
Hosts: Uri[]
/// The default is 1000 ms.
Timeout: int
/// Number of warm up ping executions. The default is 1.
WarmUpExecutions: int
/// Number of aggregated ping executions. The default is 4.
Executions: int
} with
static member CreateDefault([<ParamArray>]hosts: string[]) = {
static member CreateDefault([<ParamArray>]hosts: string[]) =
{
Hosts = hosts |> Array.map Uri
Timeout = 1_000
}
WarmUpExecutions = 1
Executions = 4
Copy link
Contributor

@AntyaDev AntyaDev Nov 18, 2021

Choose a reason for hiding this comment

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

let's simplify it
let's don't have it configurable but rather use it as a constant value
let's have 3 only invocations not 4

Copy link
Contributor Author

@deyanp deyanp Nov 18, 2021

Choose a reason for hiding this comment

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

I need to have this configurable, and to be able to increase the value, as sometimes there is a very high variability in the psping values (see my previous reply) ...

Copy link
Contributor

@AntyaDev AntyaDev Nov 18, 2021

Choose a reason for hiding this comment

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

Maybe we should run it at the start of test plus and end of the test to capture a bigger diapason?

Copy link
Contributor

@AntyaDev AntyaDev Nov 18, 2021

Choose a reason for hiding this comment

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

on the other hand, I think all this brings an accidental complexity.
This is not the primary purpose of NBomber to correctly measure PING

Copy link
Contributor

@AntyaDev AntyaDev Dec 16, 2021

Choose a reason for hiding this comment

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

Hi @deyanp
I have created a new project NBomber.Contracts
https://github.com/PragmaticFlow/NBomber/tree/v3/src/NBomber.Contracts

Copy link
Contributor Author

@deyanp deyanp Dec 16, 2021

Choose a reason for hiding this comment

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

Hi @AntyaDev , not sure what that brings, is it in relation to the discussion about Plugins depending on some InternalExtensions?

It turned out I just need these, so I copy-pasted them in my local project and managed to decouply the PsPingPlugin from the NBomber project:

namespace Framework.NBomber.Extensions

module InternalExtensions =

    let inline isNotNull (value) =
        not(isNull value)

    module Option =

        let ofRecord (value: 'T) =
            let boxed = box(value)
            if isNotNull(boxed) then Some value
            else None

and

module internal Framework.NBomber.Domain.Stats.Statistics

open System

module Constants =
    let StatsRounding = 2

module Converter =
    let inline round (digits: int) (value: float) = Math.Round(value, digits)

I have adjusted a bit the code so that there are also a few "warmup" pings at the beginning, as otherwise I was getting some crazy ping durations ... Still getting such for Azure though, not sure why, even though I am pinging from 1 AKS cluster another AKS cluster both the same vnet and region (the legendary 2ms are not achievable ;). Same setup in GCP is much more stable for some reason ...

Copy link
Contributor

@AntyaDev AntyaDev Dec 16, 2021

Choose a reason for hiding this comment

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

The point with NBomber.Contracts is that to develop any plugin; you need to add reference on NBomber.Contracts instead of the complete NBomber projects. In this way, you don't create cycle references. In other words, you fully decoupled.

Copy link
Contributor

@AntyaDev AntyaDev Dec 16, 2021

Choose a reason for hiding this comment

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

Regarding 2ms and customization around warmup - from my perspective is a good sign that something went wrong.
But ok, I explained it already several times :)

}

static member CreateDefault(hosts: string seq) =
hosts |> Seq.toArray |> PsPingPluginConfig.CreateDefault

type PsPingReply = {
Status: string
Address: Uri
RoundtripTime: int64
RequestCountSucceeded: int
RequestCountFailed: int
RoundtripTimeMin: int64
RoundtripTimeMax: int64
RoundtripTimeAvg: float
Copy link
Contributor

@AntyaDev AntyaDev Nov 18, 2021

Choose a reason for hiding this comment

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

Let's simplify it :)
Let's have only RoundtripTime where you will use avg time

Copy link
Contributor Author

@deyanp deyanp Nov 18, 2021

Choose a reason for hiding this comment

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

Well, I need all these, from home office I get something very crazy roundtrip times, and want to know about them, e.g. 2 runs done with 15 minutes difference, both from home (not sure what is happening):

image

image

Copy link
Contributor

@AntyaDev AntyaDev Nov 18, 2021

Choose a reason for hiding this comment

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

yes, but do u understand that if you have > 2 ms you can stop testing

Copy link
Contributor

@AntyaDev AntyaDev Nov 18, 2021

Choose a reason for hiding this comment

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

and I don't see a big pint to have min, max here, and ok, fail
it's PING not something critical

Copy link
Contributor Author

@deyanp deyanp Nov 18, 2021

Choose a reason for hiding this comment

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

Well, I saw the warning you have built in, and was wondering why so. My serverside request duration is between 70 and 100ms in the best case and depending on the load goes up to 300ms per request. Why would I be bothered by 50ms pings? As long as they are stable, I can safely subtract 50ms from the results of my executions, and I am fine ... e.g.:

image

Copy link
Contributor Author

@deyanp deyanp Nov 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Can you at least make it possible for having External Pluguns, not part of the NBomber Solution? Currently this seems to be impossibke, due to a dependency on an internal module ...

Copy link
Contributor Author

@deyanp deyanp Nov 18, 2021

Choose a reason for hiding this comment

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

I meant NBomber.​Extensions​.​InternalExtensions

Copy link
Contributor

@AntyaDev AntyaDev Nov 18, 2021

Choose a reason for hiding this comment

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

not sure that I understand the problem.

Copy link
Contributor Author

@deyanp deyanp Nov 18, 2021

Choose a reason for hiding this comment

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

I cannot currently extract the PsPingPlugin and have outside of the NBomber project due to this dependency ... if I could then I would have my version of PsPingPlugin as I need it, and you can have the cut-down version without min/max/stdev and Executions config.

Copy link
Contributor

@AntyaDev AntyaDev Nov 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

sure, I will take a look later at it.
if you want send PR

RoundtripTimeStdDev: float
}

module internal PsPingPluginStatistics =
Expand All @@ -43,8 +56,8 @@ module internal PsPingPluginStatistics =
let private createColumns () =
[| "Host", "Host", "System.String"
"Port", "Port", "System.Int32"
"Status", "Status", "System.String"
"Address", "Address", "System.String"
"Requests", "Requests", "System.String"
"RoundTripTime", "Round Trip Time", "System.String" |]
|> Array.map(fun x -> x |> createColumn)

Expand All @@ -53,9 +66,13 @@ module internal PsPingPluginStatistics =

row.["Host"] <- host
row.["Port"] <- port
row.["Status"] <- pingReply.Status.ToString()
row.["Address"] <- pingReply.Address.ToString()
row.["RoundTripTime"] <- $"%i{pingReply.RoundtripTime} ms"
row.["Requests"] <- $"all = {pingReply.RequestCountSucceeded + pingReply.RequestCountFailed}, ok = {pingReply.RequestCountSucceeded}, fail = {pingReply.RequestCountFailed}"
row.["RoundTripTime"] <-
$"min = {pingReply.RoundtripTimeMin}" +
$", mean = {pingReply.RoundtripTimeAvg |> Converter.round(Constants.StatsRounding)}" +
$", max = {pingReply.RoundtripTimeMax}" +
$", StdDev = {pingReply.RoundtripTimeStdDev |> Converter.round(Constants.StatsRounding)}"

row

Expand All @@ -76,10 +93,10 @@ module internal PsPingPluginHintsAnalyzer =
let analyze (pingResults: (string * int * PsPingReply)[]) =

let printHint (hostName, port, result: PsPingReply) =
$"Physical latency to host: '%s{hostName}' on port: '%i{port}' is '%d{result.RoundtripTime}'. This is bigger than 2ms which is not appropriate for load testing. You should run your test in an environment with very small latency."
$"Physical latency to host: '%s{hostName}' on port: '%i{port}' is '%f{result.RoundtripTimeAvg}'. This is bigger than 2ms which is not appropriate for load testing. You should run your test in an environment with very small latency."

pingResults
|> Seq.filter(fun (_,_,result) -> result.RoundtripTime > 2L)
|> Seq.filter(fun (_,_,result) -> result.RoundtripTimeAvg > 2.0)
|> Seq.map printHint
|> Seq.toArray

Expand All @@ -92,33 +109,52 @@ type PsPingPlugin(pluginConfig: PsPingPluginConfig) =

let execPing (config: PsPingPluginConfig) = task {
try
// from https://stackoverflow.com/questions/26067342/how-to-implement-psping-tcp-ping-in-c-sharp
use sock = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)
sock.Blocking <- true

let! replies =
let replies =
config.Hosts
|> Array.map(fun uri -> task {
let stopwatch = Stopwatch()
|> Array.map(fun uri ->
let results =
[1..config.WarmUpExecutions + config.Executions]
|> List.map(fun _ ->
// from https://stackoverflow.com/questions/26067342/how-to-implement-psping-tcp-ping-in-c-sharp
use sock = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)
sock.Blocking <- true

let stopwatch = Stopwatch()

// Measure the Connect call only
stopwatch.Start()
let connectTask = sock.ConnectAsync(uri.Host, uri.Port)
let _ = connectTask.Wait(config.Timeout) // we do not care if it completed OK or not, as the result will get anyway sock.Connected property ...
stopwatch.Stop()

// measure the Connect call only
stopwatch.Start()
let connectTask = sock.ConnectAsync(uri.Host, uri.Port)
let timeoutTask = Task.Delay(config.Timeout)
do! Task.WhenAny(connectTask, timeoutTask) |> Task.map ignore
stopwatch.Stop()
let result =
sock.Connected,
stopwatch.Elapsed.TotalMilliseconds |> int64

sock.Close()

System.Threading.Thread.Sleep(500) // to have some interval between running the tasks

result
)

let results = results |> Seq.skip config.WarmUpExecutions
let totalMsResults = results |> Seq.map (fun (_, totalMs) -> totalMs |> float)
let avg = totalMsResults |> Seq.average
let psPingReply = {
Status = if sock.Connected then "Connected" else "NotConnected/TimedOut"
Address = uri
RoundtripTime = stopwatch.Elapsed.TotalMilliseconds |> int64
RequestCountSucceeded = results |> Seq.filter (fun (connected, _) -> connected) |> Seq.length
RequestCountFailed = results |> Seq.filter (fun (connected, _) -> not connected) |> Seq.length
RoundtripTimeMin = totalMsResults |> Seq.min |> int64
RoundtripTimeMax = totalMsResults |> Seq.max |> int64
RoundtripTimeAvg = avg
RoundtripTimeStdDev =
let sumOfSquaresOfDifferences = totalMsResults |> Seq.map (fun totalMs -> (totalMs - avg) * (totalMs - avg)) |> Seq.sum
Math.Sqrt(sumOfSquaresOfDifferences / (totalMsResults |> Seq.length |> float))
}

return uri.Host, uri.Port, psPingReply
})
|> Task.WhenAll

sock.Close()
uri.Host, uri.Port, psPingReply
)

return Ok replies
with
Expand All @@ -145,20 +181,20 @@ type PsPingPlugin(pluginConfig: PsPingPluginConfig) =
_logger <- context.Logger.ForContext<PsPingPlugin>()

let config =
infraConfig.GetSection("PsPingPlugin").Get<PsPingPluginConfig>()
infraConfig.GetSection("PingPlugin").Get<PsPingPluginConfig>()
|> Option.ofRecord
|> Option.defaultValue pluginConfig

_logger.Verbose("PsPingPlugin config: @{PsPingPluginConfig}", config)
_logger.Verbose("PingPlugin config: @{PingPluginConfig}", config)

config
|> execPing
|> Task.map(createStats config)
|> Task.map(Result.map(fun (pingResults,stats) ->
|> Task.map (createStats config)
|> Task.map (Result.map(fun (pingResults,stats) ->
_pingResults <- pingResults
_pluginStats <- stats
))
|> Task.map(Result.mapError(fun ex -> _logger.Error(ex.ToString())))
|> Task.map (Result.mapError(fun ex -> _logger.Error(ex.ToString())))
|> Task.map ignore
:> Task

Expand Down