hook otel support up through clog #64
Conversation
ryanfkeepers
force-pushed
the
otel-integration
branch
from
00f2248 to
a7655de
Compare
clog/builder.go
Outdated
| } | ||
|
|
||
| if otelLog != nil { | ||
| otelLog.Emit(b.ctx, record) |
There was a problem hiding this comment.
Assuming we'll update alcion data plane with latest clues at some point, I am a little vary of turning this on by default. Have we evaluated whether this can cause any perf impact? Also, have we tested if initializing an otel endpoint inside ecs works? If not, I'd prefer we make otel hook-in optional. What do you think?
There was a problem hiding this comment.
Ah I am guessing we won't call clues.Initialize in alcion data plane, so otel endpoint will never be stood up.
There was a problem hiding this comment.
Assuming we'll update alcion data plane with latest clues at some point
Those updates can be frozen and/or use the (now archived) clog repo, instead of the clues repo. There's really no reason to bump the package with us on maintenance mode. Exception cases can be handled as they arise.
guessing we won't call clues.Initialize in alcion data plane
In the case that we do bump clues for alcion, yes, that's definitely the easy way out.
Adds otel support for logging via clog. Any clog log will automatically duplicate the log via otel logging. We may want deduplication in the future, which will take more effort to support a better interface for logger plug-in. That way we could configure a run for zap, otel, or otel-via-zap.
ryanfkeepers
force-pushed
the
clog-otel-intg
branch
from
43b1af2 to
e662932
Compare
Adds otel support for logging via clog.
Any clog log will automatically duplicate the log via otel logging. We may want deduplication in the future, which will take more effort to support a better interface for logger plug-in. That way we could configure a run for zap, otel, or otel-via-zap.