Skip to content

Fix tracing decorator with late configuration#2754

Merged
lzchen merged 11 commits intoopen-telemetry:mainfrom
oceyral:fix_tracing_decorator_with_late_config
Jul 6, 2022
Merged

Fix tracing decorator with late configuration#2754
lzchen merged 11 commits intoopen-telemetry:mainfrom
oceyral:fix_tracing_decorator_with_late_config

Conversation

@oceyral
Copy link
Copy Markdown
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
Copy Markdown

linux-foundation-easycla bot commented Jun 10, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@ocelotl
Copy link
Copy Markdown
Contributor

ocelotl commented Jun 13, 2022

@oceyral please sign the CLA

@oceyral
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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