Skip to content

Commit

Permalink
Fix tracing module sampling option should default to 1.0 when not set (
Browse files Browse the repository at this point in the history
…#3231)

This commit ensures that if the `sampling` option of the tracing
module's client is not provided by user, the Client defaults to
sample 100% of the request.
  • Loading branch information
oleiade committed Jul 31, 2023
1 parent 3c006a5 commit 7184786
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 4 deletions.
52 changes: 52 additions & 0 deletions cmd/tests/tracing_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,58 @@ func TestTracingModuleClient_HundredPercentSampling(t *testing.T) {
assertHasTraceIDMetadata(t, jsonResults, 100, tb.Replacer.Replace("HTTPBIN_IP_URL/tracing"))
}

func TestTracingModuleClient_NoSamplingSetShouldAlwaysSample(t *testing.T) {
t.Parallel()
tb := httpmultibin.NewHTTPMultiBin(t)

var gotRequests int64
var gotSampleFlags int64

tb.Mux.HandleFunc("/tracing", func(w http.ResponseWriter, r *http.Request) {
atomic.AddInt64(&gotRequests, 1)

traceparent := r.Header.Get("traceparent")
require.NotEmpty(t, traceparent)
require.Len(t, traceparent, 55)

if traceparent[54] == '1' {
atomic.AddInt64(&gotSampleFlags, 1)
}
})

script := tb.Replacer.Replace(`
import http from "k6/http";
import { check } from "k6";
import tracing from "k6/experimental/tracing";
export const options = {
// 100 iterations to make sure we get 100% sampling
iterations: 100,
}
// We do not set the sampling option, thus the default
// behavior should be to always sample.
const instrumentedHTTP = new tracing.Client({
propagator: "w3c",
})
export default function () {
instrumentedHTTP.get("HTTPBIN_IP_URL/tracing");
};
`)

ts := getSingleFileTestState(t, script, []string{"--out", "json=results.json"}, 0)
cmd.ExecuteWithGlobalState(ts.GlobalState)

assert.Equal(t, int64(100), atomic.LoadInt64(&gotSampleFlags))
assert.Equal(t, int64(100), atomic.LoadInt64(&gotRequests))

jsonResults, err := fsext.ReadFile(ts.FS, "results.json")
require.NoError(t, err)

assertHasTraceIDMetadata(t, jsonResults, 100, tb.Replacer.Replace("HTTPBIN_IP_URL/tracing"))
}

func TestTracingModuleClient_ZeroPercentSampling(t *testing.T) {
t.Parallel()
tb := httpmultibin.NewHTTPMultiBin(t)
Expand Down
13 changes: 10 additions & 3 deletions js/modules/k6/experimental/tracing/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (mi *ModuleInstance) newClient(cc goja.ConstructorCall) *goja.Object {
//
// When used in the context of a k6 script, it will automatically replace
// the imported http module's methods with instrumented ones.
func (mi *ModuleInstance) instrumentHTTP(options options) {
func (mi *ModuleInstance) instrumentHTTP(options goja.Value) {
rt := mi.vu.Runtime()

if mi.vu.State() != nil {
Expand All @@ -109,10 +109,17 @@ func (mi *ModuleInstance) instrumentHTTP(options options) {
common.Throw(rt, err)
}

// Parse the options instance from the JS value.
// This will also validate the options, and set the sampling
// rate to 1.0 if the option was not set.
opts, err := newOptions(rt, options)
if err != nil {
common.Throw(rt, fmt.Errorf("unable to parse options object; reason: %w", err))
}

// Initialize the tracing module's instance default client,
// and configure it using the user-supplied set of options.
var err error
mi.Client, err = NewClient(mi.vu, options)
mi.Client, err = NewClient(mi.vu, opts)
if err != nil {
common.Throw(rt, err)
}
Expand Down
2 changes: 1 addition & 1 deletion js/modules/k6/experimental/tracing/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func newOptions(rt *goja.Runtime, from goja.Value) (options, error) {
}

fromSamplingValue := from.ToObject(rt).Get("sampling")
if fromSamplingValue == nil || common.IsNullish(fromSamplingValue) {
if common.IsNullish(fromSamplingValue) {
opts.Sampling = defaultSamplingRate
}

Expand Down

0 comments on commit 7184786

Please sign in to comment.