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

Add an extra statement in function to give autometrics context #78

Closed
gagbo opened this issue Nov 2, 2023 · 0 comments · Fixed by #79
Closed

Add an extra statement in function to give autometrics context #78

gagbo opened this issue Nov 2, 2023 · 0 comments · Fixed by #79

Comments

@gagbo
Copy link
Member

gagbo commented Nov 2, 2023

Giving out the context created by autometrics.PreInstrument is the best (only?) way to enable library users to pass the context down to the function's callees if they wish to.

Having autometrics context through a complete call tree will allow to have cleaner traces support, and better caller information for the call graph feature.

We could also reuse part of the middleware code, to shadow the function argument context with the autometrics one if we are able to make a type-compatible variant.

gagbo added a commit that referenced this issue Nov 2, 2023
The parent tracking is span-based (using `(TraceID, SpanId)` as keys)

It falls back to the previous method (`runtime.Callers()` analysis) if
the function id for a given span is missing from the global state.

To prevent the global state from ballooning memory usage, the entries
in the span-to-functionIDs table are deleted as part of the cleanup
code in `Instrument` (that is why some goroutines outliving their
callers with weird scheduling might miss the function id in the
global state.)

This solution deals well with functions spawning multiple goroutines,
but the caller info will be "enhanced" (i.e. with Autometrics annotated
functions in the parent field) only if the context created by
autometrics.PreInstrument is properly passed to callees, which is
a task followed in #78

Fixes: AM-42
gagbo added a commit that referenced this issue Nov 3, 2023
This allow library users to reuse the context augmented with Autometrics, so the tracing and caller information can be a lot more precise

Fixes: #78
gagbo added a commit that referenced this issue Nov 3, 2023
This allow library users to reuse the context augmented with Autometrics, so the tracing and caller information can be a lot more precise

Fixes: #78
gagbo added a commit that referenced this issue Nov 3, 2023
This allow library users to reuse the context augmented with Autometrics, so the tracing and caller information can be a lot more precise

Fixes: #78
gagbo added a commit that referenced this issue Nov 3, 2023
The parent tracking is span-based (using `(TraceID, SpanId)` as keys)

It falls back to the previous method (`runtime.Callers()` analysis) if
the function id for a given span is missing from the global state.

To prevent the global state from ballooning memory usage, the entries
in the span-to-functionIDs table are deleted as part of the cleanup
code in `Instrument` (that is why some goroutines outliving their
callers with weird scheduling might miss the function id in the
global state.)

This solution deals well with functions spawning multiple goroutines,
but the caller info will be "enhanced" (i.e. with Autometrics annotated
functions in the parent field) only if the context created by
autometrics.PreInstrument is properly passed to callees, which is
a task followed in #78

Fixes: AM-42
gagbo added a commit that referenced this issue Nov 3, 2023
This allow library users to reuse the context augmented with Autometrics, so the tracing and caller information can be a lot more precise

Fixes: #78
@gagbo gagbo linked a pull request Nov 6, 2023 that will close this issue
3 tasks
@gagbo gagbo closed this as completed in #79 Nov 13, 2023
gagbo added a commit that referenced this issue Nov 13, 2023
#79)

* Add repository fields to BuildInfo metric

Fixes AM-41

* Add system to track only Autometricized parents

The parent tracking is span-based (using `(TraceID, SpanId)` as keys)

It falls back to the previous method (`runtime.Callers()` analysis) if
the function id for a given span is missing from the global state.

To prevent the global state from ballooning memory usage, the entries
in the span-to-functionIDs table are deleted as part of the cleanup
code in `Instrument` (that is why some goroutines outliving their
callers with weird scheduling might miss the function id in the
global state.)

This solution deals well with functions spawning multiple goroutines,
but the caller info will be "enhanced" (i.e. with Autometrics annotated
functions in the parent field) only if the context created by
autometrics.PreInstrument is properly passed to callees, which is
a task followed in #78

Fixes: AM-42

* Split context generation from defer statement

This allow library users to reuse the context augmented with Autometrics, so the tracing and caller information can be a lot more precise

Fixes: #78

* Better, shorter error messages

* Accept more function signatures in instrument

Instead of erroring out when some arguments cannot be inspected for context inspection, we log the information.

* Add more build information in binaries

This will fix the builds that are downloaded, but not the ones from go install

* Fix context replacement for http.Request

The context of a HTTP request is not assignable, so we need to read from the existing context and create a new one

* Add autometrics.version to build_info metric
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 a pull request may close this issue.

1 participant