https://authzed.com logo
Join Discord
Powered by
Does that approach sound right? O r do i
# spicedb
t
Does that approach sound right? O r do i need to check into anything else ....if it sounds good then do I proceed with coding it down and push a commit when its done
y
yeah, the ask is basically to figure out why the otel flags by themselves are insufficient and fix that issue. if we can do that, we should deprecate/hide the 
--otel-provider
flag and update the documentation accordingly
what you're describing sounds about right
t
hey, pushed a PR for #712
the fix ensures we don’t skip OTel initialization when only OTEL_ env vars are set, so the SDK can configure itself properly
kept it minimal and avoided overriding SDK behavior
please do tell me if any changes are needed
y
yep, i'll have a look in the next day or two
thank you for putting it up!
t
Hey, I finished tracing the full initialization path across SpiceDB, cobrautil, and cobraotel and I think I have the issue narrowed down pretty cleanly now. Main findings: I could not reproduce a propagation bug in cobraotel itself env -> Viper -> Cobra propagation works correctly in tests SpiceDB DefaultPreRunE ordering is also correct I did confirm two separate issues though: Confirmed bug in cobrautil Inside SyncViperPreRunE, a local Viper instance is created but SetEnvPrefix is called on the global singleton instead of the local instance: viper.SetEnvPrefix(prefix) -> should be: v.SetEnvPrefix(prefix) This does not appear to break #712 directly because BindEnv later uses explicit env var names, but it is still unintended global state mutation and should probably be fixed independently. Confirmed design gap in cobraotel Tracing initialization exits immediately whenever otel-provider == "none" which is also the default value. That means: OTEL_EXPORTER_OTLP_ENDPOINT alone cannot activate tracing SPICEDB_OTEL_ENDPOINT alone cannot activate tracing explicit provider activation is mandatory This seems to be the actual source of confusion behind #712. Why OTEL_EXPORTER_OTLP_ENDPOINT appeared to help: the underlying OTel SDK reads OTEL_* vars independently, but cobraotel still requires provider activation before initialization proceeds. Proposed follow-ups: small PR for the cobrautil singleton fix separate discussion/PR around whether cobraotel should auto-enable tracing when OTEL_* exporter config exists, or at minimum emit a warning/document the behavior more clearly I also wrote a local test suite covering the full propagation and initialization flow if that would be useful for a PR.
the whole thing took me on a pretty good rabbit hole if i may call it so , i shall update the whole journey onto a notion notebook now so that we may refer to it later for any particular information
y
awesome, i'll take a look! thank you for digging into this
we found another issue with cobraotel, which is that traces don't get flushed because the signal handling logic doesn't have a handle on the otelprovider and therefore can't call `Flush`: https://github.com/authzed/spicedb/issues/3095
so our current thought is to move off of it and essentially do the same logic in SpiceDB directly
t
that makes sense and is an interesting way to proceed
this will also let us bring lifecycle management completely under spice dbs control
the signal handling gap makes sense given how cobraotel currently owns the provider without exposing it
happy to work on it
My plan would be to replicate the provider initialization logic directly in SpiceDB, wire the returned TracerProvider into the existing signal/shutdown handling so Shutdown() gets called on SIGTERM/SIGINT, and drop the cobraotel dependency
Does that match what you had in mind, or are there constraints on the shutdown path I should know about first?
y
yep, that sounds good to me
and we can iterate on it if needed
thank you!
t
Hey! The OTel lifecycle PR is nearly ready to open. Quick question before I do — for the 
--otel-provider
flag, you mentioned we should deprecate or hide it. Should I use 
MarkDeprecated
(which prints a warning to users when they pass the flag) or 
MarkHidden
(silently removes it from help output but still works)? I want to match whatever direction you had in mind before I push.
and i pushed a pr on it , https://github.com/authzed/spicedb/pull/3108 i closed and reopened too fast coz i commited the vendor directory by mistake so i got locked out the convo in git , idk if you need to unlock it or if i need to just wait for it to unlock so that i can enter the description , sorry for the extra work
y
i'm not so sure on this point after reading the code and thinking about it more. we'd still want some way to say "this is enabled" or "this is not enabled," and I think that flag would be fine.
and no worries, i'll take a look on monday
t
Okayyy thankss
I'll keep --otel-provider as-is, no deprecation or hiding for now and we can iteraew if needed
y
awesome. thanks!
i said this in the PR as well, but I'd rather keep most of this logic out of the cobra layer - most components of SpiceDB have some configuration object that the flags are copied into, and then the object is constructed as needed in the run logic
we want to do the same here
t
Suree , will look into it and update you by tomorrow
Made the changes and repplied to the comments , hope i was helpful
y
cool, will check. thank you!
t
Heyy , just wanted to check if the pr is still relevant or if I should look into something else , whichever do let me know 😊
21 Views
PreviousNext
Powered by