search

temporalio / sdk-core

Core Temporal SDK that can be used as a base for language specific Temporal SDKs
MIT License
262 stars 70 forks source link

core: make opentelemetry dependencies optional #690

Closed djc closed 5 months ago

djc commented 6 months ago

What was changed

Make the telemetry features optional but enabled by default in the core SDK.

This is a first take -- happy to take feedback and iterate.

Why?

As discussed in #687, it would be nice to limit dependencies. When using the Rust SDK, the application might have its own tracing subscriber setup, and it would be better for the libraries not to interfere with that.

Checklist

  1. See #687
  2. How was this tested: it still compiles
  3. Any docs updates needed: not sure?
djc commented 5 months ago

Good suggestion about the module, I think this looks a lot cleaner.

Sushisource commented 5 months ago

@djc This needs to merge from master or rebase. I would do it for you but, same deal I mentioned before: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

There should be a way to turn that on so I can easily do things like that

djc commented 5 months ago

I looked at the docs last time and didn't find the option. I can probably fix it up myself in a bit once my daughter is in bed.

djc commented 5 months ago

This is what it looks like for me:

Screenshot 2024-03-01 at 20 09 25
Sushisource commented 5 months ago

Weird. I don't get why the option isn't showing up for you. That's a bit obnoxious.