Skip to content
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

Tracing improvements #2962

Merged
merged 2 commits into from
Jun 16, 2022
Merged

Tracing improvements #2962

merged 2 commits into from
Jun 16, 2022

Conversation

rhafer
Copy link
Contributor

@rhafer rhafer commented Jun 14, 2022

Create separate TracerProviders per service to improve tracing when
running multiple services in a single process (like e.g. ocis does).
Previously all service were referring to the same package-level
TracerProvider (which, among other things, caused the services to
overwrite each others services names).

We still create a "global" TracerProvider to cover the cases where
we don't have access to the per service Provider yet. This will be
improved in a follow up change.

@rhafer rhafer requested review from a team, labkode, ishank011 and glpatcern as code owners June 14, 2022 12:54
@update-docs
Copy link

update-docs bot commented Jun 14, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

Create separate TracerProviders per service to improve tracing when
running multiple services in a single process (like e.g. ocis does).
Previously all service were referring to the same package-level
TracerProvider (which, among other things, caused the services to
overwrite each others services names).

We still create a "global" TracerProvider to cover the cases where
we don't have access to the per service Provider yet. This will be
improved in a follow up change.
To generate more useful tracing (especially when running multiple
services in a single process) we'd like to generate Tracers from the
per service TracerProvider() instead of the global default provider.
Therefore we now pass the TracerProvider via context (similar to what
is already done for the logger.
@rhafer
Copy link
Contributor Author

rhafer commented Jun 15, 2022

I must admit I am not really sure about the whole change (especially the second commit in the PR). I am not a fan of attaching this kind of stuff to the context. I think ideally we would instantiate a tracer per "sub"-service at startup and utilized that. (That would however require quite a bit of rework I fear)

@rhafer
Copy link
Contributor Author

rhafer commented Jun 15, 2022

To be really useful with ocis theres also a small change required on the ocis side to initialize the DefaultTracerProvider: owncloud/ocis@master...rhafer:tracing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants