Add Percentile support #86

zhiburt · 2022-10-07T13:58:23Z

Hi there

I doubt I've done the right thing...
But I tried.

close #72

Signed-off-by: Maxim Zhiburt <[email protected]>

zhiburt · 2022-10-07T14:32:56Z

I guess it should consider all scenarious?

kursataktas

Thank you @zhiburt for this work. I've added my comments.

I think we don't need to display percentiles on realtime data.
I guess p99, p95, p90, and p80 would be enough for now.
Let's add a new flag to display/calculate percentiles as optional. Since percentile calculation would be time-consuming.

Example (CLI):
ddosify -t target.com --output-percentile

Example (Config JSON):

{
    "request_count": 10,
    "load_type": "linear",
    "duration": 5,
    "output": "stdout-json",
    "output_percentile": true,
    ...
}

You may need to change the Init() contract of the Report service to pass the new flag to report service implementations.

Example outputs on different scenarios;

1 Step stdout
Multi Step stdout
1 Step stdout-json

{
    "success_perc": 100,
    "fail_perc": 0,
    "success_count": 10,
    "fail_count": 0,
    "avg_duration": 0.225,
    "percentiles": {
          "p99": 0.123,
          "p95": 0.321,
          "p90": 0.33,
          "p80": 0.51
     },
    "steps": {
      "1": {
        "name": "D-1",
        "status_code_dist": {
          "200": 10
        },
        "error_dist": {
          
        },
        "durations": {
          "connection": 0.009,
          "dns": 0.001,
          "request_write": 0,
          "response_read": 0,
          "server_processing": 0.095,
          "tls": 0.01,
          "total": 0.116
        },
        "percentiles": {  // all percentiles are calculated on durations.total value.
          "p99": 0.123,
          "p95": 0.321,
          "p90": 0.33,
          "p80": 0.51
        },
        "success_count": 10,
        "fail_count": 0,
        "success_perc": 100,
        "fail_perc": 0
      }
    }
  }

Multi Step stdout-json

{
  "success_perc": 100,
  "fail_perc": 0,
  "success_count": 10,
  "fail_count": 0,
  "avg_duration": 0.225,
  "total_duration_percentiles": {
        "p99": 0.123,
        "p95": 0.321,
        "p90": 0.33,
        "p80": 0.51
   },
  "steps": {
    "1": {
      "name": "D-1",
      "status_code_dist": {
        "200": 10
      },
      "error_dist": {
        
      },
      "durations": {
        "connection": 0.009,
        "dns": 0.001,
        "request_write": 0,
        "response_read": 0,
        "server_processing": 0.095,
        "tls": 0.01,
        "total": 0.116
      },
      "total_duration_percentiles": {  // all percentiles are calculated on durations.total value.
        "p99": 0.123,
        "p95": 0.321,
        "p90": 0.33,
        "p80": 0.51
      },
      "success_count": 10,
      "fail_count": 0,
      "success_perc": 100,
      "fail_perc": 0
    },
    "2": {
      "name": "D-2",
      "status_code_dist": {
        "200": 10
      },
      "error_dist": {
        
      },
      "durations": {
        "connection": 0.008,
        "dns": 0.001,
        "request_write": 0,
        "response_read": 0,
        "server_processing": 0.09,
        "tls": 0.009,
        "total": 0.109
      },
      "total_duration_percentiles": {  // all percentiles are calculated on durations.total value.
        "p99": 0.123,
        "p95": 0.321,
        "p90": 0.33,
        "p80": 0.51
      },
      "success_count": 10,
      "fail_count": 0,
      "success_perc": 100,
      "fail_perc": 0
    }
  }
}

I wonder what you think about these?

core/report/aggregator.go

…tile

Signed-off-by: Maxim Zhiburt <[email protected]>

zhiburt · 2022-10-18T21:23:36Z

Everything must be addressed.

kursataktas

Thank you for this new design. It looks great 💯

I put my words for the related sections
There are some failed tests, we have to take a look at them
We should add more tests to cover new changes
We may want to update the example_config to show the usage.

kursataktas · 2022-10-19T12:33:59Z

config/json.go

@@ -182,6 +183,7 @@ func (j *JsonReader) CreateHammer() (h types.Hammer, err error) {
 		Scenario:          s,
 		Proxy:             p,
 		ReportDestination: j.Output,
+		ReportPercentiles: j.OutputPercentile,


Some places use "Percentile" others use "Percentiles". I think we can use "Percentile" everywhere.

Or "Percentiles". Whatever you want, but let's make the naming consistent

kursataktas · 2022-10-19T12:40:01Z

main.go

@@ -74,6 +74,8 @@ var (
 	certPath    = flag.String("cert_path", "", "A path to a certificate file (usually called 'cert.pem')")
 	certKeyPath = flag.String("cert_key_path", "", "A path to a certificate key file (usually called 'key.pem')")

+	outputPercentile = flag.Bool("output-percentile", false, "Report percentile")


"Report percentile" --> "Report percentile(P99, P95, P90, P80)"

kursataktas · 2022-10-19T12:54:06Z

core/report/stdout.go

-		red(fmt.Sprintf("%s Failed Run: %-6d %3d%% %5s",
-			emoji.CrossMark, s.result.FailedCount, s.result.failedPercentage(), "")),
-		blue(fmt.Sprintf("%s  Avg. Duration: %.5fs", emoji.Stopwatch, s.result.AvgDuration)))
+		green(fmt.Sprintf("%s  Successful Run: %-6d %3d%% %5s", emoji.CheckMark, s.result.SuccessCount, s.result.successPercentage(), "")),


The max. line-length is limited to 120 chars as stated in the linter settings (at .golangci.yml). Let's follow the standards.

kursataktas · 2022-10-21T12:50:46Z

core/report/aggregator.go


+	for _, report := range result.ItemReports {
+		for key, duration := range report.Durations {
+			report.TotalDurations[key] = append(report.TotalDurations[key], duration)


We don't need to store other durations (DNS, TLS, etc.) total duration would be enough ("duration") for now.

kursataktas · 2022-10-21T12:53:23Z

core/report/aggregator.go


+	for _, report := range result.ItemReports {


We don't need this extra loop.

We can add below code snippet at line 57 or anywhere in that else statement

if percentileReportEnabled { item.TotalDurations["duration"] = append(item.TotalDurations["duration"], float32(rr.Duration.Seconds())) }

I think the aggregator should store durations only if the percentage report is enabled.

kursataktas · 2022-10-21T14:19:54Z

core/report/aggregator.go

+
+	n := int(math.Round((float64(p) / 100.0) * float64(len(list))))
+	if n > 0 {
+		// I am not sure about the case where n == 0


No need to cover this unless we need a percentile less than p50, we can remove this check.

kursataktas · 2022-10-21T15:26:16Z

core/report/stdoutJson.go

+
+		if s.reportPercentiles {
+			jsonResult.ItemReports[key].Percentiles = map[string]float32{
+				"p99": item.DurationPercentile(99),


Milliseconds would be enough.

Current stdout-json example:

... "percentiles": { "p80": 0.12210735, --> "0.122" would be enough "p90": 0.14628321, "p95": 0.1905153, "p99": 0.1905153 }, ...

kursataktas · 2022-10-21T15:33:54Z

core/report/stdoutJson.go

+			FailedCount:    item.FailedCount,
+		}
+
+		if s.reportPercentiles {


If possible, let's not show the percentiles key if it is not enabled.

Current state:

... "durations": { "connection": 0.005, "dns": 0.009, "request_write": 0, "response_read": 0.382, "server_processing": 0.253, "tls": 0.012, "total": 0.661 }, "percentiles": null, ---> Let's remove this if we can "success_count": 10, "fail_count": 0 }, ...

zhiburt added 2 commits October 7, 2022 16:56

Add Percentile support

e165ab9

Signed-off-by: Maxim Zhiburt <[email protected]>

Merge scenarius when doing p(95)

2cb7be8

Signed-off-by: Maxim Zhiburt <[email protected]>

kursataktas reviewed Oct 17, 2022

View reviewed changes

core/report/aggregator.go Outdated Show resolved Hide resolved

zhiburt added 3 commits October 18, 2022 22:23

Merge branch 'master' of github.com:ddosify/ddosify into patch-percen…

da6968d

…tile

Report percentile only on report stage + add CLI flags

97cbdd4

Signed-off-by: Maxim Zhiburt <[email protected]>

Fix ordering of percentiles in stdout

6a39bcd

Signed-off-by: Maxim Zhiburt <[email protected]>

zhiburt requested a review from kursataktas October 19, 2022 06:30

kursataktas reviewed Oct 21, 2022

View reviewed changes

fatihbaltaci marked this pull request as draft December 6, 2022 16:14

pelohsoe approved these changes Sep 19, 2023

View reviewed changes

fatihbaltaci closed this Aug 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Percentile support #86

Add Percentile support #86

zhiburt commented Oct 7, 2022

zhiburt commented Oct 7, 2022

kursataktas left a comment •

edited

Loading

zhiburt commented Oct 18, 2022 •

edited

Loading

kursataktas left a comment

kursataktas Oct 19, 2022

kursataktas Oct 21, 2022

kursataktas Oct 19, 2022

kursataktas Oct 19, 2022

kursataktas Oct 21, 2022

kursataktas Oct 21, 2022

kursataktas Oct 21, 2022 •

edited

Loading

kursataktas Oct 21, 2022

kursataktas Oct 21, 2022

kursataktas Oct 21, 2022 •

edited

Loading

Add Percentile support #86

Add Percentile support #86

Conversation

zhiburt commented Oct 7, 2022

zhiburt commented Oct 7, 2022

kursataktas left a comment • edited Loading

Choose a reason for hiding this comment

zhiburt commented Oct 18, 2022 • edited Loading

kursataktas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kursataktas Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kursataktas Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

kursataktas left a comment •

edited

Loading

zhiburt commented Oct 18, 2022 •

edited

Loading

kursataktas Oct 21, 2022 •

edited

Loading

kursataktas Oct 21, 2022 •

edited

Loading