Conversation
|
|
||
| @Override | ||
| public void onEmit(Context context, ReadWriteLogRecord logRecord) { | ||
| if (Loopback.isLoopback(context)) { |
There was a problem hiding this comment.
We also have to check in instrumentation for this I suppose?
There was a problem hiding this comment.
Yes exactly, for example: https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/15572/files#diff-19adc31cb9699cb43b2c7701d9838a060a4ae299710b1e0b382493cfd1cc382f
There's another version of this we can consider as well:
- Update the internals of
SdkLoggerProvidersuch every call toLogRecordProcessor#onEmit(Context, ReadWriteLogRecord)hasContextw/otel.loopback=true(implicit and explicit context) - Update the internals of
SdkLoggerProvidersuch that calls toSdkLogRecordBuilder#emit()short circuit ifotel.loopback=true
I went with the approach you see here because modifying the internals of the SDK is arguably spec territory and this approach allows solving without changing internals.
Also, even if SdkLogRecordBuilder#emit() is short circuiting if otel.loopback=true, you'd still want instrumentation to short circuit to avoid the unnecessary mapping work which involves memory allocation.
But it might be worth to do both things. Or to start with this type of solution which doesn't rely on modying SDK internal, and later evolve if something lands in the spec.
2 participants
See open-telemetry/opentelemetry-java-instrumentation#15572