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

Fix tracing module sampling option should default to 1.0 when not set #3231

Merged
merged 1 commit into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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