-
Notifications
You must be signed in to change notification settings - Fork 96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: allow converting otel trace ids to datadog values #91
Conversation
Alright, after some function optimization one of my coworkers came up with a better solution. Benchmarks:
|
Some how missed charlist encoding not working. Should be fixed now. Working again on our side. |
Alright, it's been in production for a bit now. No errors that I can see and from a quick spot test, traces and logs are being correlated correctly. No noticeable performance hit for memory or CPU. |
@btkostner thank you for another great PR ❤️. I'm moving between houses now and will be able to look more closely/merge it in a day or two, sorry for the delay. Also, I got an idea I decided to discuss with you. I feel like a change like this one can be generalized: with the new commit that added This will ensure that no matter what use case you have - you can always integrate OTEL or anything else that writes keys to the Logger metadata and you can control how it's encoded and under which key is stored. The issue with such implementation is that if the function is not available when we are trying to write the log - the logger would crash, so using it would be unsafe. I will check if there is anything we can do to prevent this. I'm thinking to implement such refactoring after we merge this PR. What do you think? |
No rush on the review. Take your time. As for something like |
Sorry for delay, a rocket hit near my parents house, then my friends house and I'm under the weather for some time. But I've started looking into adding a new option we discussed and then I'll release a new version. |
No rush at all! Let me know if you want some assistance on adding metadata mappers and I can make some time. |
This is to allow Open Telemetry trace ids to be converted into Datadog format so logs and traces can be linked together. Datadog has a page on how to do this. Most of the code was stollen from the golang example, and then the test values were stollen from known good versions based off that go code. You'll notice that the code is very defensive in case some bad value gets there and starts breaking things.
I'll also mention that this is probably not the most optimized version. I'm not great with binary pattern matching so improvements welcome.Currently this is what would show up out of the box in Datadog with https://github.com/open-telemetry/opentelemetry-erlang setup.
![image](https://user-images.githubusercontent.com/3385679/192855067-edf40cb0-9e4b-47bf-b525-1d06b889e63a.png)
Note I'm going to be running this locally for a bit on a project in production to ensure nothing breaks and works as intended.