Skip to content

Fix tracing decorator with late configuration - #2754

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

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

Conversation

@oceyral

@oceyral oceyral commented Jun 10, 2022

Copy link
Copy Markdown
Contributor

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

linux-foundation-easycla Bot commented Jun 10, 2022

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

@ocelotl

ocelotl commented Jun 13, 2022

Copy link
Copy Markdown
Contributor

@oceyral please sign the CLA

@oceyral

oceyral commented Jun 13, 2022

Copy link
Copy Markdown
Contributor Author

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

@oceyral

oceyral commented Jun 28, 2022

Copy link
Copy Markdown
Contributor Author

@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

ocelotl commented Jun 30, 2022

Copy link
Copy Markdown
Contributor

@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