Skip to content

fix(profiling): make sure lineno is always an int and funcname is never None #3099

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

Merged
merged 1 commit into from
Jan 18, 2022

Conversation

jd
Copy link
Contributor

@jd jd commented Dec 31, 2021

In some cases, f_lineno can be None which would trigger an error in the pprof
output. This is not even caught by mypy, and I've proposed a fix here:

python/typeshed#6769

This patch makes sure we always use 0 if the line number is None.

It also makes sure a funcname is always passed — in practice it's now the case,
but make sure typing reflects that.

Fixes #3046

@jd jd requested a review from a team as a code owner December 31, 2021 14:41
@jd jd added the changelog/no-changelog A changelog entry is not required for this PR. label Jan 3, 2022
Copy link
Contributor

@P403n1x87 P403n1x87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because unfortunately the f_lineno attribute can be 0 at times, the "correct" way of computing the line number is via f_lasti and f_code.co_lnotab. The details are in https://github.com/python/cpython/blob/main/Objects/lnotab_notes.txt. The annoying bit is that I believe the formula described there has changed a couple of times across releases.

@jd
Copy link
Contributor Author

jd commented Jan 17, 2022

Because unfortunately the f_lineno attribute can be 0 at times, the "correct" way of computing the line number is via f_lasti and f_code.co_lnotab. The details are in https://github.com/python/cpython/blob/main/Objects/lnotab_notes.txt. The annoying bit is that I believe the formula described there has changed a couple of times across releases.

That's something we could improve in the future, though it does not impact this PR AFAICT.

@jd jd force-pushed the profiling/pprof-fix branch from d60879b to e251ca7 Compare January 17, 2022 13:32
@jd jd changed the title fix(profiling): make sure lineno is always an int fix(profiling): make sure lineno is always an int and funcname is never None Jan 17, 2022
…er None

In some cases, f_lineno can be `None` which would trigger an error in the pprof
output. This is not even caught by mypy, and I've proposed a fix here:

  python/typeshed#6769

This patch makes sure we always use 0 if the line number is `None`.

It also makes sure a funcname is always passed — in practice it's now the case,
but make sure typing reflects that.

Fixes DataDog#3046
@jd jd force-pushed the profiling/pprof-fix branch from e251ca7 to 4173b8b Compare January 17, 2022 13:43
@jd jd requested review from brettlangdon and P403n1x87 January 17, 2022 14:24
@Kyle-Verhoog Kyle-Verhoog merged commit 7d5b446 into DataDog:master Jan 18, 2022
@jd jd deleted the profiling/pprof-fix branch January 19, 2022 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/no-changelog A changelog entry is not required for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: '<' not supported between instances of 'NoneType' and 'str'
4 participants