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

CP-50444: Instrument http svr with dt #5888

Merged

Conversation

GabrielBuica
Copy link
Contributor

@GabrielBuica GabrielBuica commented Jul 29, 2024

Instruments functions such as:

handle_one
callback
callback1
dispatch
jsoncallback
response
...

This should help us determine where the time is spent between receiving the request and sending back a response.

Latest BVT passed: 202675 (Dev Run)

@GabrielBuica GabrielBuica force-pushed the private/dbuica/CP-50444 branch 2 times, most recently from 99fbc72 to f63ea3d Compare July 29, 2024 11:09
@psafont
Copy link
Member

psafont commented Jul 31, 2024

This should fix the issue until we implement a better abstraction for the span hashtables.

Also known as 'never' ;)

@GabrielBuica
Copy link
Contributor Author

image
For reference, this is how the trace looks.

ocaml/libs/http-lib/dune Show resolved Hide resolved
Adds `with_child_trace` fuction that only creates a span only if there
exists a parent.

Signed-off-by: Gabriel Buica <danutgabriel.buica@cloud.com>
Intruments the following path of an http request:

- `handle_one`,
- `callback`,
- `callback1`,
- `dispatch`,
- `jsoncallback`,
- `response`.

Signed-off-by: Gabriel Buica <danutgabriel.buica@cloud.com>
@GabrielBuica
Copy link
Contributor Author

Passes BVT 202354.

@GabrielBuica
Copy link
Contributor Author

Last commit updates the parent so that the trace looks like this:

image

ocaml/libs/tracing/tracing.ml Outdated Show resolved Hide resolved
ocaml/libs/tracing/tracing.ml Outdated Show resolved Hide resolved
Intruments the request reading loop.

Adds new functionality to the tracing library to update a span with a
new parent.

This way we can retroactively set the parent span with the correnct one.

Signed-off-by: Gabriel Buica <danutgabriel.buica@cloud.com>
Fixes warning of unused records fields.

These fields are part of the opentelementry spec but we are currently
not using them in `xapi`.

Signed-off-by: Gabriel Buica <danutgabriel.buica@cloud.com>
Increases the default maximum number of spans inside a trace from `1000`
to `2500`.

Now that we instrumented the internal http calls, with all components
enabled, the number of spans inside `xapi` component for a `vm-start`
operations is slightly  greater than `1000`. This causes spans to be
leaked, they are removed from the ongoing span table but never added in
the finished tabled. Therefore, they are lost unless the limit is
change in `/etc/xapi.conf`.

This should fix the issue until we implement a better abstraction for
the span hashtables.

Signed-off-by: Gabriel Buica <danutgabriel.buica@cloud.com>
….Request`

Like the title says, this commit adds `Http.Request.with_tracing` to
`Http.Request`.

This should enable updating the traceparent field of a request while we
process it.

Signed-off-by: Gabriel Buica <danutgabriel.buica@cloud.com>
@last-genius last-genius merged commit cc66500 into xapi-project:master Aug 9, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants