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.

Sign up for GitHub

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

Experimental tracing module (3/3): define and expose a instrumentHTTP function #2855

Merged
merged 9 commits into from
Jan 26, 2023
Merged

Experimental tracing module (3/3): define and expose a instrumentHTTP function #2855

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
31d317d
Expose the instrumentHTTP function as part of the module's public API
oleiade Jan 13, 2023
22f1f70
Add integration tests covering the instrumentHTTP function
oleiade Jan 13, 2023
2ff5246
Update cmd/tests/tracing_module_test.go
oleiade Jan 25, 2023
7f858e9
Update cmd/tests/tracing_module_test.go
oleiade Jan 25, 2023
7163425
Add unit tests for the instrumentHTTP function
oleiade Jan 26, 2023
60e1e3b
Remove unnecessary integration tests
oleiade Jan 26, 2023
7d468aa
Add integration test illustrating instrumentHTTP in multi-file setup
oleiade Jan 26, 2023
d5616dc
Move unit test setup in dedicated struct
oleiade Jan 26, 2023
7187e12
Adjust test ensuring instrumenting function from other module works
oleiade Jan 26, 2023
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
253 changes: 222 additions & 31 deletions cmd/tests/tracing_module_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,37 +56,7 @@ func TestTracingModuleClient(t *testing.T) {
jsonResults, err := afero.ReadFile(ts.FS, "results.json")
require.NoError(t, err)

gotHTTPDataPoints := false

for _, jsonLine := range bytes.Split(jsonResults, []byte("\n")) {
if len(jsonLine) == 0 {
continue
}

var line sampleEnvelope
require.NoError(t, json.Unmarshal(jsonLine, &line))

if line.Type != "Point" {
continue
}

// Filter metric samples which are not related to http
if !strings.HasPrefix(line.Metric, "http_") {
continue
}

gotHTTPDataPoints = true

anyTraceID, hasTraceID := line.Data.Metadata["trace_id"]
require.True(t, hasTraceID)

traceID, gotTraceID := anyTraceID.(string)
require.True(t, gotTraceID)

assert.Len(t, traceID, 32)
}

assert.True(t, gotHTTPDataPoints)
assertHasTraceIDMetadata(t, jsonResults)
}

func TestTracingClient_DoesNotInterfereWithHTTPModule(t *testing.T) {
Expand Down Expand Up @@ -128,6 +98,227 @@ func TestTracingClient_DoesNotInterfereWithHTTPModule(t *testing.T) {
assert.Equal(t, int64(2), atomic.LoadInt64(&gotInstrumentedRequests))
}

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

var gotRequests int64

tb.Mux.HandleFunc("/tracing", func(w http.ResponseWriter, r *http.Request) {
atomic.AddInt64(&gotRequests, 1)
assert.NotEmpty(t, r.Header.Get("traceparent"))
assert.Len(t, r.Header.Get("traceparent"), 55)
})

script := tb.Replacer.Replace(`
import http from "k6/http";
import tracing from "k6/experimental/tracing";

tracing.instrumentHTTP({
propagator: "w3c",
})

export default function () {
http.del("HTTPBIN_IP_URL/tracing");
http.get("HTTPBIN_IP_URL/tracing");
http.head("HTTPBIN_IP_URL/tracing");
http.options("HTTPBIN_IP_URL/tracing");
http.patch("HTTPBIN_IP_URL/tracing");
http.post("HTTPBIN_IP_URL/tracing");
http.put("HTTPBIN_IP_URL/tracing");
http.request("GET", "HTTPBIN_IP_URL/tracing");
};
`)

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

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

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

assertHasTraceIDMetadata(t, jsonResults)
}

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

var gotRequests int64

tb.Mux.HandleFunc("/tracing", func(w http.ResponseWriter, r *http.Request) {
atomic.AddInt64(&gotRequests, 1)
assert.NotEmpty(t, r.Header.Get("uber-trace-id"))
assert.Len(t, r.Header.Get("uber-trace-id"), 45)
})

script := tb.Replacer.Replace(`
import http from "k6/http";
import { check } from "k6";
import tracing from "k6/experimental/tracing";

tracing.instrumentHTTP({
propagator: "jaeger",
})

export default function () {
http.del("HTTPBIN_IP_URL/tracing");
http.get("HTTPBIN_IP_URL/tracing");
http.head("HTTPBIN_IP_URL/tracing");
http.options("HTTPBIN_IP_URL/tracing");
http.patch("HTTPBIN_IP_URL/tracing");
http.post("HTTPBIN_IP_URL/tracing");
http.put("HTTPBIN_IP_URL/tracing");
http.request("GET", "HTTPBIN_IP_URL/tracing");
};
`)

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

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

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

assertHasTraceIDMetadata(t, jsonResults)
}

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

var gotRequests int64

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

assert.NotEmpty(t, r.Header.Get("traceparent"))
assert.Len(t, r.Header.Get("traceparent"), 55)

assert.NotEmpty(t, r.Header.Get("X-Test-Header"))
assert.Equal(t, "test", r.Header.Get("X-Test-Header"))
})

script := tb.Replacer.Replace(`
import http from "k6/http";
import tracing from "k6/experimental/tracing";

tracing.instrumentHTTP({
propagator: "w3c",
})

const testHeaders = {
"X-Test-Header": "test",
}

export default function () {
http.del("HTTPBIN_IP_URL/tracing", null, { headers: testHeaders });
http.get("HTTPBIN_IP_URL/tracing", { headers: testHeaders });
http.head("HTTPBIN_IP_URL/tracing", { headers: testHeaders });
http.options("HTTPBIN_IP_URL/tracing", null, { headers: testHeaders });
http.patch("HTTPBIN_IP_URL/tracing", null, { headers: testHeaders });
http.post("HTTPBIN_IP_URL/tracing", null, { headers: testHeaders });
http.put("HTTPBIN_IP_URL/tracing", null, { headers: testHeaders });
http.request("GET", "HTTPBIN_IP_URL/tracing", null, { headers: testHeaders });
};
`)

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

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

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

assertHasTraceIDMetadata(t, jsonResults)
Comment on lines +232 to +237
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of the same comment about this not checking that all of those have tracing ids.

Maybe assertHasTraceIDMetadata can have a number argument that is how many it expects 🤷.

Or to take the expected url(s) (as vararg not required argument) and check that all of those have trace id metadata sample.

}

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

script := tb.Replacer.Replace(`
import tracing from "k6/experimental/tracing";

export default function() {
tracing.instrumentHTTP({
propagator: "w3c",
})
}
`)

ts := getSingleFileTestState(t, script, []string{}, 0)
ts.ExpectedExitCode = 0
cmd.ExecuteWithGlobalState(ts.GlobalState)

assert.Contains(t, ts.Stderr.String(), "instrumentHTTP can only be called in the init context")
}

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

script := tb.Replacer.Replace(`
import tracing from "k6/experimental/tracing";

tracing.instrumentHTTP({
propagator: "w3c",
})

tracing.instrumentHTTP({
propagator: "w3c",
})

export default function() {
}
`)

ts := getSingleFileTestState(t, script, []string{}, 0)
ts.ExpectedExitCode = 107
cmd.ExecuteWithGlobalState(ts.GlobalState)

assert.Contains(t, ts.Stderr.String(), "instrumentHTTP can only be called once")
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of prefer if those are not integration tests as they are more expensive and in this case there is no need for them IMO.

The functionality tested is pretty common :

  1. exception in an iteration does not abort the whole test
  2. an exception in the init context - does.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me see if I can easily turn them into standard unit tests indeed. I've struggled with instrumentHTTP depending on require in the past, but with the changes with made, it might not be such an issue anymore 👍🏻


// assertHasTraceIDMetadata checks that the trace_id metadata is present and has the correct format
// for all http metrics in the json results file.
func assertHasTraceIDMetadata(t *testing.T, jsonResults []byte) {
gotHTTPDataPoints := false

for _, jsonLine := range bytes.Split(jsonResults, []byte("\n")) {
if len(jsonLine) == 0 {
continue
}

var line sampleEnvelope
require.NoError(t, json.Unmarshal(jsonLine, &line))

if line.Type != "Point" {
continue
}

// Filter metric samples which are not related to http
if !strings.HasPrefix(line.Metric, "http_") {
continue
}

gotHTTPDataPoints = true

anyTraceID, hasTraceID := line.Data.Metadata["trace_id"]
require.True(t, hasTraceID)

traceID, gotTraceID := anyTraceID.(string)
require.True(t, gotTraceID)

assert.Len(t, traceID, 32)
}

assert.True(t, gotHTTPDataPoints)
}

// sampleEnvelope is a trimmed version of the struct found
// in output/json/wrapper.go
// TODO: use the json output's wrapper struct instead if it's ever exported
Expand Down
70 changes: 69 additions & 1 deletion js/modules/k6/experimental/tracing/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ type (
// ModuleInstance represents an instance of the JS module.
ModuleInstance struct {
vu modules.VU

// Client holds the module's default tracing client.
*Client
oleiade marked this conversation as resolved.
Show resolved Hide resolved
}
)

Expand Down Expand Up @@ -45,7 +48,8 @@ func (*RootModule) NewModuleInstance(vu modules.VU) modules.Instance {
func (mi *ModuleInstance) Exports() modules.Exports {
return modules.Exports{
Named: map[string]interface{}{
"Client": mi.newClient,
"Client": mi.newClient,
"instrumentHTTP": mi.instrumentHTTP,
},
}
}
Expand All @@ -69,3 +73,67 @@ func (mi *ModuleInstance) newClient(cc goja.ConstructorCall) *goja.Object {

return rt.ToValue(NewClient(mi.vu, opts)).ToObject(rt)
}

// InstrumentHTTP instruments the HTTP module with tracing headers.
//
// 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) {
rt := mi.vu.Runtime()

if mi.vu.State() != nil {
common.Throw(rt, common.NewInitContextError("tracing module's instrumentHTTP can only be called in the init context"))
}

if mi.Client != nil {
oleiade marked this conversation as resolved.
Show resolved Hide resolved
err := errors.New(
"tracing module's instrumentHTTP can only be called once. " +
"if you were attempting to reconfigure the instrumentation, " +
"please consider using the tracing.Client instead",
)
common.Throw(rt, err)
}

// Initialize the tracing module's instance default client,
// and configure it using the user-supplied set of options.
mi.Client = NewClient(mi.vu, options)

// Explicitly inject the http module in the VU's runtime.
// This allows us to later on override the http module's methods
// with instrumented ones.
httpModuleValue, err := rt.RunString(`require('k6/http')`)
if err != nil {
common.Throw(rt, err)
}

httpModuleObj := httpModuleValue.ToObject(rt)

// Closure overriding a method of the provided imported module object.
//
// The `onModule` argument should be a *goja.Object obtained by requiring
// or importing the 'k6/http' module and converting it to an object.
//
// The `value` argument is expected to be callable.
mustSetHTTPMethod := func(method string, onModule *goja.Object, value interface{}) {
// Inject the new get function, adding tracing headers
// to the request in the HTTP module object.
err = onModule.Set(method, value)
if err != nil {
common.Throw(
rt,
fmt.Errorf("unable to overwrite http.%s method with instrumented one; reason: %w", method, err),
)
}
}

// Overwrite the implementation of the http module's method with the instrumented
// ones exposed by the `tracing.Client` struct.
mustSetHTTPMethod("del", httpModuleObj, mi.Client.Del)
mustSetHTTPMethod("get", httpModuleObj, mi.Client.Get)
mustSetHTTPMethod("head", httpModuleObj, mi.Client.Head)
mustSetHTTPMethod("options", httpModuleObj, mi.Client.Options)
mustSetHTTPMethod("patch", httpModuleObj, mi.Client.Patch)
mustSetHTTPMethod("post", httpModuleObj, mi.Client.Patch)
mustSetHTTPMethod("put", httpModuleObj, mi.Client.Patch)
mustSetHTTPMethod("request", httpModuleObj, mi.Client.Request)
}
Loading