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

Fix tracing decorator with late configuration #2754

Merged

Conversation

oceyral
Copy link
Contributor

@oceyral oceyral commented Jun 10, 2022

Description

Related to #2621 and #2708

When using the tracing decorator documented as part of the issue and PR above, functions might not actually be instrumented when the decorator is used on a function before a tracing provider is configured.

Considering the following case :


from opentelemetry import trace
from opentelemetry.sdk.trace import TracerProvider


tracer = trace.get_tracer(__name__)

def setup_tracing() -> None:
    provider = TracerProvider()
    processor = BatchSpanProcessor(ConsoleSpanExporter())
    provider.add_span_processor(processor)
    trace.set_tracer_provider(provider)

@tracer.start_as_current_span("toto")
def toto():
    print("toto")

def main():
    setup_tracing()
    toto()

if __name__ == '__main__':
    main()

Here, the toto function will not actually be instrumented, even after configuration of a tracing provider. This is particularly problematic with instrumenting submodules for example, because if the module is imported before a tracer provider is configured, most uses of the decorator will be no-ops, and no tracing will be done.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Does This PR Require a Contrib Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@oceyral oceyral requested a review from a team June 10, 2022 10:54
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 10, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@ocelotl
Copy link
Contributor

ocelotl commented Jun 13, 2022

@oceyral please sign the CLA

@oceyral
Copy link
Contributor Author

oceyral commented Jun 13, 2022

@ocelotl yes, I'm trying to navigate corporate structure to get validated as a contributor, sorry if it takes a bit of time

@oceyral
Copy link
Contributor Author

oceyral commented Jun 28, 2022

@ocelotl got the CLA signed ! I also tried fixing mypy/lint tests, but I'm not sure about the other failing tests, I had a look but it doesn't look related to my PR.

@ocelotl
Copy link
Contributor

ocelotl commented Jun 30, 2022

@ocelotl got the CLA signed ! I also tried fixing mypy/lint tests, but I'm not sure about the other failing tests, I had a look but it doesn't look related to my PR.

Thanks, seems like this PR and others are failing because an unrelated infra issue which should be fixed by #2790.

@lzchen lzchen merged commit dbcec9f into open-telemetry:main Jul 6, 2022
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.

4 participants