Skip to content

Comments

add support for callgraph profiling#686

Open
seemk wants to merge 9 commits intomainfrom
callgraphs
Open

add support for callgraph profiling#686
seemk wants to merge 9 commits intomainfrom
callgraphs

Conversation

@seemk
Copy link

@seemk seemk commented Jan 23, 2026

New environment variables:

  • SPLUNK_SNAPSHOT_PROFILER_ENABLED default: false
  • SPLUNK_SNAPSHOT_SELECTION_PROBABILITY default: 0.01
  • SPLUNK_SNAPSHOT_SAMPLING_INTERVAL default: 10 (milliseconds)

The profiling is triggered in CallgraphsSpanProcessor which filters the stacktraces based on active traces (i.e. it only keeps stacktraces matching a trace that has been selected for profiling).

Changes to profiling:

  • Add the missing profiling.instrumentation.source attribute.
  • Use time.monotonic instead of time.time for consistent sleeping.
  • Add the ability to pause profiling - which hibernates the profiler thread. Can be resumed again via start. This is used for callgraphs when no traces have been selected for a minute.
  • Add support for multiple profiler instances. Context attach/detach wrapping is still done only once, meaning the profiler instances share the thread state mapping.

@seemk seemk requested review from a team as code owners January 23, 2026 12:55
Copy link
Contributor

@pmcollins pmcollins left a comment

Choose a reason for hiding this comment

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

Hi Siim! This is great. Added some comments.

Copy link
Contributor

@pmcollins pmcollins left a comment

Choose a reason for hiding this comment

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

Just a nit and a question about shutdown.

Comment on lines +30 to +33
if not parent_span.is_valid:
return True

return parent_span.is_remote
Copy link
Contributor

Choose a reason for hiding this comment

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

For clarity, what do you think about this instead:

    is_root_span = not parent_span.is_valid
    return is_root_span or parent_span.is_remote 

self._profiler.pause_after(60.0)

def shutdown(self) -> None:
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the profiler be shut down here?

Copy link
Contributor

@pmcollins pmcollins left a comment

Choose a reason for hiding this comment

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

LGTM. Maybe the shutdown thing is not an issue because it's generally called atexit when the profiler thread will be shutdown anyway. The other comment is just about readability.

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