#sooo say i'm inside of a `go test`
stone viper
Yeah there's not a way to do that in userspace w/out passing it in via an env var, forcibly including the .git dir, etc. Those would create excessive cache invalidation (i.e. cache is invalidated by git commit even if nothing in the tests changed between the two commits), but that doesn't really matter atm.
I wonder if we should just automatically include git metadata as otel attributes, if we don't already
Like in this case it would be adding those attrs transparently in the engine, so userspace doesn't need to do anything
lucid atlas
I wonder if we should just automatically include git metadata as otel attributes...
i like the sound of this. part of what i'm stepping through right now is just the UX of userspace obs extensions, and so while it's easy enough to plumb shit through myself via env var, i'm looking for it to happen automagically and without cache invalidation.
from the engine otel server perspective, though, idk how easy it's gonna be to catch those incoming spans/etc and correctly associate a container/sha/etc
but i'll peek at it tomorrow 🙂
OTEL_RESOURCE_ATTRIBUTES does exist so that bodes well for a generic x-language solution at least being feasible
approaching it from that engine angle though... is @unborn jewel 's question gonna make shit wacky? like if i'm running an exogenous module against my code, whose git info do we set in the resource attrs?
glossy brook
I wonder if we should just automatically include git metadata as otel attributes...
AFAIK we're already attaching some vcs metadat to the trace / spans. Haven't checked the code but I'm wondering if we might want to be leveraging onthe OTel Baggage API for this maybe https://opentelemetry.io/docs/concepts/signals/baggage/.
cc @sand lantern
unborn jewel
But we're attaching the wrong vcs metadata (from workdir on the host rather than module source).
glossy brook
Hmm I didn't get that Connor was expecting to get the module source git metadata from his comment
He mentioned "git metadata about the repository". He didn't specify if that was either the module source or the user's app code
unborn jewel
Yeah I'm not sure either. But just took the opportunity for a reminder that we are sending the wrong git metadata 😛
lucid atlas
app code, not module code, but idk if there's situations where that can get confused (imagine a module that runs go test on user code)
lucid atlas
AFAIK we're already attaching some vcs metadat to the trace / spans. Haven't che...
that's a great point, baggage might make this fairly straightforward to propogate the vcs data downward (rather than the awkward injection thing i had hinted at)
unborn jewel
I think "git metadata" should always be qualified. When you attach it unqualified to a span, it's ambiguous what it actually means. And, a given span may have a relationship to different git repositories.
For example, the span for a withExec operation:
- Git info for the current module
- Git info for the workdir on the host (what we send currently)
- Git info for the workdir in the container
- Git info for any directory currently copied or mounted in the container
--> We dont have to send all that information with the span necessarily, but we should have a nomenclature that makes it possible to do so, and removes ambiguity as to what you're looking at when inspecting the span
lucid atlas
agreed.
Git info for the workdir on the host (what we send currently)
Git info for the workdir in the container
this distinction isn't one i've tried to draw before, though.... how do you end up with these 2 being different things?
atm we strip out the git metadata as we pass the workdir through, so git info for the workdir inside the container is... elusive
unborn jewel
agreed.
> Git info for the workdir on the host (what we send currently)
> Git ...
$ git clone https://github.com/foo/bar
$ cd bar
$ dagger shell -c 'container | from alpine | with-directory . $(git https://github.com/dagger/dagger | head | tree) | with-exec -- ls -l`
That last ls -l has a workdir inside github.com/dagger/dagger. Meanwhile the host workdir is inside the completely unrelated github.com/foo/bar
lucid atlas
Ah yeah makes sense tq
lucid atlas
@glossy brook @stone viper so digging a little on userspace otel attribute propogation, i think it's in the spirit of the otel spec to use OTEL_RESOURCE_ATTRIBUTES here- they're immutable, sent once per batch, and configured once on OTeL setup within the userspace process.
baggage would get the job done in my isolated case, but has a bit more faff for the userspace process to configure correctly in order to pick it up, and then gets sent with literally every span, log, metric AND propogated across the wire for network calls... that means if you have a test running in dagger with VCS baggage configured by dagger, and then it makes outgoing calls to your prod services which also pick up baggage, your prod traces are gonna have dev details in them like committer name, etc that are associated with the tests' dagger VCS information without any clear attr naming setup to indicate where that came from... this gets even more unpleasant if you start thinking about a x-workdir, daggerized integration test setup, like a service built on the fly from 1 repo and then tests from another... the tests will try to baggage their VCS information into the service and then everything is pretty instantaneously nonsensical
resourceattrs stay manageable and are basically properties of a single otel-enabled process, not an entire trace, and i'm fairly certain that's what users would expect here. for attrs that you do wanna propogate down the entire trace, these resourceattrs could be popped out of the context and rekeyed into baggage as appropriate, like users might choose to remap baggage[userwebsite.com/testclient.git.ref] = resourceattrs[dagger.io/git.ref]
i also wanna say out loud that i'm not rabbitholing here entirely uncritically lol, i'm sneakily laying groundwork for dagger cloud to one day have automated, otel-driven flake detection 😈
unborn jewel
that would be pretty amazing
lucid atlas
it's my pet white whale of a CI feature lol
glossy brook
@lucid atlas I skimmed the code yesterday and I did find that we seem to already be leveraging resource attributes to store git metadata here: https://github.com/marcosnils/dagger/blob/8c0cefb0e8d3f16307e7c93c2a11e9095d501fed/cmd/dagger/main.go?plain=1#L273
having said that, I'm not really sure how/if this info can be retrieved within a module
lucid atlas
you tagged the wrong connor lol, i've got 2 ns.
that code is setting the resource attrs of the dagger CLI, not propogating them down into userspace via OTEL_RESOURCE_ATTRIBUTES. one interesting element regarding the propagation is that those attributes may not ever be sent to the engine, and the engine is the bit that'd need to set up that env var for userspace processes.