feat: major api improvements

[GregoryAlbouy]

Description

Introduction of major changes to the SDK for a more intuitive and convenient usage.

Usage

Minimal configuration: before

package main

import (
    "context"
    "fmt"

    "github.com/benchttp/engine/runner"
)

func main() {
    // Retrieve a configuration object first
    config := runner.DefaultConfig()
    config.Request = config.Request.WithURL("http://localhost:3000") // tedious, especially with URL being required

    // Instantiate runner and run benchmark
    report, _ := runner. // package name "runner" is too generic
        New(nil). // awkward signature (gets worse with a non nil value)
        Run(context.Background(), config)

    fmt.Println(report.Metrics.ResponseTimes.Mean)
}

Minimal configuration: after

package main

import (
    "context"
    "fmt"

    "github.com/benchttp/sdk/benchttp"
)

func main() {
    report, _ := benchttp.
        DefaultRunner().
        WithNewRequest("GET", "http://localhost:3000", nil).
        Run(context.Background())

    fmt.Println(report.Metrics.ResponseTimes.Mean)
}

Advanced configuration: before

package main

import (
    "context"
    "fmt"
    "net/url"
    "time"

    "github.com/benchttp/engine/runner"
)

func main() {
    payload := []byte(`{"a": "b"}`)
    uri, _ := url.Parse("http://localhost:3000")

    config := runner.Config{
        Request: runner.RequestConfig{
            Method: "POST",
            URL:    uri,
            Body:   runner.NewRequestBody("raw", string(payload)), // 🤢
        },
        Runner: runner.RecorderConfig{ // what is runner.RecorderConfig?
            Requests:       100,
            Concurrency:    10,
            RequestTimeout: 2 * time.Second,
            GlobalTimeout:  30 * time.Second,
        },
    }

    // weird name avoiding naming conflicts
    // weird runner.New signature
    rnr := runner.New(func(progress runner.RecordingProgress) {
        fmt.Println(progress)
    })

    report, _ := rnr.Run(context.Background(), config)

    fmt.Println(report.Metrics.ResponseTimes.Mean)
}

Advanced configuration: after

package main

import (
    "bytes"
    "context"
    "fmt"
    "net/http"
    "time"

    "github.com/benchttp/sdk/benchttp"
)

func main() {
    payload := []byte(`{"a": "b"}`)
    request, _ := http.NewRequest("POST", "http://localhost:3000", bytes.NewReader(payload))

    runner := benchttp.Runner{
        Request:        request,
        Requests:       100,
        Concurrency:    10,
        RequestTimeout: 2 * time.Second,
        GlobalTimeout:  30 * time.Second,
        OnProgress: func(progress benchttp.RecordingProgress) {
            fmt.Println(progress)
        },
    }

    report, _ := runner.Run(context.Background())

    fmt.Println(report.Metrics.ResponseTimes.Mean)
}

Changes

Global changes

• Change module name: github.com/benchttp/engine ➡️ github.com/benchttp/sdk. It implies to change this repository name to sdk after this PR is merged.
• Change package name: runner ➡️ benchttp: more intuitive naming, such as runner := benchttp.Runner{} or report := benchttp.Report{} or errors.Is(err, benchttp.ErrCanceled)
• Remove Makefile in favor to shell scripts
• Global clean-up: remove irrelevant env file, .gitignore entries, deprecated linters, unused deps

runner

• Remove Config overriding logics. Same results can be achieved by changing signatures from ToConfig(v interface{}) Config to ToConfig(dst *Config), which significantly reduces complexity.
• Remove ConfigFields types and values: they're became unnecessary after removal of Config overriding logics
• Change Config.Request type to *http.Request: the runner needs an actual request, not an intermediary representation to be compiled as a request.
• Flatten packages config, report
• Remove the notion of Config, use Runner fields instead
• Rename Runner.onRecordingProgress to Runner.OnProgress (public field)
• Remove superfluous constructor New: no internal setup needed, public fields are enough.

configparse

• Change parsing functions signature and implementation so they store the result into a given structure (à la json.Unmarshal(b []byte, dst interface{})) . This has two advantages:
  1. Directly overrides the relevant fields of the given dst: no need to remember assigned fields then use Config overriding logics
  2. Allows to use any runner configuration as a base, rather than an imposed zero/default value

Notes

[codecov-commenter]

Codecov Report

Merging #58 (4c84242) into main (43a24f9) will increase coverage by 0.52%.
The diff coverage is 50.92%.

❗ Current head 4c84242 differs from pull request most recent head ced71cd. Consider uploading reports for the commit ced71cd to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main      #58      +/-   ##
==========================================
+ Coverage   73.88%   74.40%   +0.52%     
==========================================
  Files          28       25       -3     
  Lines         873      801      -72     
==========================================
- Hits          645      596      -49     
+ Misses        203      181      -22     
+ Partials       25       24       -1     

Flag	Coverage Δ
unittests	74.40% <50.92%> (+0.52%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
benchttp/internal/metrics/aggregate.go	94.73% <ø> (ø)
benchttp/internal/metrics/compare.go	100.00% <ø> (ø)
benchttp/internal/metrics/field.go	100.00% <ø> (ø)
benchttp/internal/metrics/metrics.go	88.23% <ø> (ø)
benchttp/internal/metrics/timestats/sort.go	100.00% <ø> (ø)
benchttp/internal/metrics/timestats/timestats.go	96.42% <ø> (ø)
benchttp/internal/recorder/error.go	100.00% <ø> (ø)
benchttp/internal/recorder/httputil.go	100.00% <ø> (ø)
benchttp/internal/recorder/progress.go	29.41% <ø> (ø)
benchttp/internal/recorder/recorder.go	88.54% <ø> (ø)
... and 12 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[moreirathomas]

Very nice, it brings much needed simplicity 👍

Dropping the Config struct make the whole thing much lighter.

Single concern I have: now that we configure the Runner via its public fields, nothing prevents the user from mutating its values after initialization. I agree that's a stupid thing to do from them, yet doable and I think we can manage an api that prevents this. That's on the todo list I think, you're refactor already brings value that largely outweights this single drawback (for now?).

As it's been quite a long time, I suggest we simply merge, what do you think?

[GregoryAlbouy]

Single concern I have: now that we configure the Runner via its public fields, nothing prevents the user from mutating its values after initialization. I agree that's a stupid thing to do from them, yet doable and I think we can manage an api that prevents this.

My intuition is that it would be difficult to achieve without losing simplicity (and Go users are used to it, e.g. with http.Request), but worth discussing indeed!