-
Notifications
You must be signed in to change notification settings - Fork 164
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
Setters and getters for trace IDs #967
base: main
Are you sure you want to change the base?
Conversation
This pull request was exported from Phabricator. Differential Revision: D60267172 |
This pull request was exported from Phabricator. Differential Revision: D60267172 |
Summary: Pull Request resolved: pytorch#967 This commit moves the trace ID initialization logic to be inline with how ActivityProfilers interact with LoggerObservers by setting a default local trace ID when a profiler config does not contain a trace ID. The USTLoggerCollector, which is our internal LoggerObserver will record the trace ID for a given environment (which is a PID today) using the `setTraceID` method that all LoggerObservers must staisfy. Additionally any internal calls to read this trace ID, for example from our ManifoldChromeTrace logger, may use a special `getTraceID` method that ships with `USTLoggerCollector`. Group trace IDs are handled accordingly. Differential Revision: D60267172
0a223b3
to
c3c9c74
Compare
This pull request was exported from Phabricator. Differential Revision: D60267172 |
Summary: Pull Request resolved: pytorch#967 This commit moves the trace ID initialization logic to be inline with how ActivityProfilers interact with LoggerObservers by setting a default local trace ID when a profiler config does not contain a trace ID. The USTLoggerCollector, which is our internal LoggerObserver will record the trace ID for a given environment (which is a PID today) using the `setTraceID` method that all LoggerObservers must staisfy. Additionally any internal calls to read this trace ID, for example from our ManifoldChromeTrace logger, may use a special `getTraceID` method that ships with `USTLoggerCollector`. Group trace IDs are handled accordingly. Differential Revision: D60267172
c3c9c74
to
9624e01
Compare
This pull request was exported from Phabricator. Differential Revision: D60267172 |
Summary: Pull Request resolved: pytorch#967 This commit moves the trace ID initialization logic to be inline with how ActivityProfilers interact with LoggerObservers by setting a default local trace ID when a profiler config does not contain a trace ID. The USTLoggerCollector, which is our internal LoggerObserver will record the trace ID for a given environment (which is a PID today) using the `setTraceID` method that all LoggerObservers must staisfy. Additionally any internal calls to read this trace ID, for example from our ManifoldChromeTrace logger, may use a special `getTraceID` method that ships with `USTLoggerCollector`. Group trace IDs are handled accordingly. Differential Revision: D60267172
9624e01
to
41a8948
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary: Pull Request resolved: pytorch#967 This commit moves the trace ID initialization logic to be inline with how ActivityProfilers interact with LoggerObservers by setting a default local trace ID when a profiler config does not contain a trace ID. The USTLoggerCollector, which is our internal LoggerObserver will record the trace ID for a given environment (which is a PID today) using the `setTraceID` method that all LoggerObservers must staisfy. Additionally any internal calls to read this trace ID, for example from our ManifoldChromeTrace logger, may use a special `getTraceID` method that ships with `USTLoggerCollector`. Group trace IDs are handled accordingly. Differential Revision: D60267172
This pull request was exported from Phabricator. Differential Revision: D60267172 |
41a8948
to
3ee82a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forget if this diff was refactored somewhere, accepting to unblock.
Summary:
This commit moves the trace ID initialization logic to be inline with how ActivityProfilers interact with LoggerObservers by setting a default local trace ID when a profiler config does not contain a trace ID.
The USTLoggerCollector, which is our internal LoggerObserver will record the trace ID for a given environment (which is a PID today) using the
setTraceID
method that all LoggerObservers must staisfy. Additionally any internal calls to read this trace ID, for example from our ManifoldChromeTrace logger, may use a specialgetTraceID
method that ships withUSTLoggerCollector
.Group trace IDs are handled accordingly.
Differential Revision: D60267172