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

[OpenTelemetry] osquery-go trace instrumentation #1244

Merged

Conversation

RebeccaMahany
Copy link
Contributor

@RebeccaMahany RebeccaMahany commented Jun 30, 2023

Pulls in changes from osquery/osquery-go#110 and adds a small amount of instrumentation to trace codepaths from osquery-go to launcher and vice versa.

Screenshots below --

Call => Distributed => GetQueries Screenshot 2023-06-30 at 11 20 04 AM
Call => Config Screenshot 2023-06-30 at 11 21 18 AM
Call => Table => exec Screenshot 2023-06-30 at 11 37 06 AM

cmd/grpc.ext/grpc.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@RebeccaMahany RebeccaMahany marked this pull request as ready for review July 7, 2023 13:26
@RebeccaMahany RebeccaMahany force-pushed the becca/osquery-trace-instrumentation branch from ee36a6d to 6e65358 Compare July 7, 2023 15:50
@@ -15,6 +15,7 @@ import (
"github.com/kolide/launcher/pkg/agent/types"
"github.com/kolide/launcher/pkg/backoff"
"github.com/kolide/launcher/pkg/osquery"
osquerygotraces "github.com/osquery/osquery-go/traces"
Copy link
Contributor

Choose a reason for hiding this comment

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

Seeing this, makes me think we should put SetTracerProvider to being in the toplevel osquery-go library. and leave traces as effectively internal. But 🤷 maybe not important

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't feel strongly about it either way

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's worth fixing now, but it feels wrong. traces should really be an internal package, not part of the exposed API surface. But 🤷 we're most of the users, and I don't feel like we need to maintain some high standard of API consistency, so not worth it.

@RebeccaMahany RebeccaMahany merged commit 510036a into kolide:main Jul 10, 2023
@RebeccaMahany RebeccaMahany deleted the becca/osquery-trace-instrumentation branch July 10, 2023 14:50
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.

3 participants