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

feat: Multiple Executions for PsPingPlugin #434

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

Conversation

deyanp
Copy link
Contributor

@deyanp deyanp commented Nov 18, 2021

@AntyaDev check this out (especially the naming of colums/values in PsPingPluginStatistics)

@deyanp
Copy link
Contributor Author

deyanp commented Nov 18, 2021

@AntyaDev , I have 2 additional questions:

  1. If PsPingPlugin has multiple executions, should PingPlugin be left with only 1?
  2. When you calculate the statistics for the actual load tests, do you automatically subtract from all min/max/mean the ping durations? Asking because initially I thought that is what was happening when the ping plugin is enabled ...

@Pragmaticflow2021
Copy link

@deyanp look, Ping is just a plugin that tests physical latency.
NBomber load test doesn't know details about ping latency vs. processing time. Therefore, it measures the execution of steps that you define. Your step can do only CPU work, and NBomber will be able to handle it tool.

RequestCountFailed: int
RoundtripTimeMin: int64
RoundtripTimeMax: int64
RoundtripTimeAvg: float
Copy link
Contributor

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

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

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

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

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

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

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

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

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

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

Hosts = hosts |> Array.map Uri
Timeout = 1_000
}
Executions = 4
Copy link
Contributor

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

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

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

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

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

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

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

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 :)

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.

3 participants