Lalit Kumar Bhasin
Lalit Kumar Bhasin
> Looking pretty good. Just a question to the code owners if they'd prefer to split the isahc removal into a separate PR. I'm fine with doing either. Thanks @aumetra...
> > Looking pretty good. Just a question to the code owners if they'd prefer to split the isahc removal into a separate PR. I'm fine with doing either. >...
@aumetra Is the upgrade to `opentelemetry-proto` submodule intended/required? If not, can we remove it from this PR, to be handled separately.
> @lalitb are you planning a release? Need help with that? @djc yes I believe @cijothomas plans to do a release today if there are no issues.
Had a quick look into the Java implementation - it's uses a [ComponentRegistry](https://github.com/open-telemetry/opentelemetry-java/blob/30d16eb3d393f85d99175a62ed246e369e25b906/sdk/common/src/main/java/io/opentelemetry/sdk/internal/ComponentRegistry.java#L41) to maintain loggers/meters/tracers, which internally uses [ConcurrentHashMap](https://docs.oracle.com/javase%2F8%2Fdocs%2Fapi%2F%2F/java/util/concurrent/ConcurrentHashMap.html) for storage. Looking further how other languages are doing it,...
I am more inclined towards `Option 2`. `Option 1` - Using different loggers for each `target`, means we need to maintain these loggers in some (thread-safe) hashmap, and the insertion/retrieval...
Also the result of stress test. There is perf improvement as we no longer do clone in log_emitter.rs. **Existing**: ``` Number of threads: 16 Throughput: 36,961,600 iterations/sec Throughput: 37,929,800 iterations/sec...
**Benchmark** | Test Name | Main | PR | |--------------------------------------------------|---------------------------------|--------------------------------| | simple-log/no-context | [128.97 ns] | [89.746 ns] | | simple-log/with-context time | [130.56 ns] | [89.683 ns] | |...
> Would the below statements be accurate? Nicely summarised the changes :) Yes to all.
Have added unit-test to validate LogProcessor chaining and updating of LogData. This is ready for review now. Also added changelog and benchmark in the description.