Add Profiler code samples #2027
Conversation
|
@tswast sounds like I can't add reviewers. Could you take a look at this PR? We'd like to add some code samples for https://cloud.google.com/profiler. Also, could we request write access to this repo? @aalexand @kalyanac could you also take a look? This change adds a quickstart sample that works for GCE, GKE and local environment. App Engine samples are to be added. |
|
|
||
| # [END profiler_python_quickstart] | ||
| while True: | ||
| pass |
There was a problem hiding this comment.
The flame graph is probably going to look quite boring. Would it make sense to make it more interesting by at least having a named function?
There was a problem hiding this comment.
Added some functions so that the profile shows CPU time percentage as expected.
profiler/quickstart/main.py
Outdated
| # Profiler initialization, best done as early as possible. | ||
| try: | ||
| googlecloudprofiler.start( | ||
| service='service-name', |
There was a problem hiding this comment.
In https://github.com/GoogleCloudPlatform/golang-samples/blob/master/profiler/profiler_quickstart/main.go we specify more realistic service name (and no service version) rather than placeholder string. Should we try to be consistent?
There was a problem hiding this comment.
Updated to use a more realistic servce name. This will be used as the code snippet for using Python agent. I slightly prefer keeping the service version, like this: https://github.com/GoogleCloudPlatform/golang-samples/blob/master/profiler/snippets/snippets.go#L28?
profiler/quickstart/main.py
Outdated
| # project_id='my-project-id', | ||
| ) | ||
| except (ValueError, NotImplementedError) as e: | ||
| print(e) # Handles errors here |
There was a problem hiding this comment.
Can you provide some explanation for when ValueError or NotImplementedError might occur?
There was a problem hiding this comment.
It's a little long and it will be presented in the doc surrounding this code snippet. I'm a little hesitate to make this code snippet too long.
profiler/quickstart/main.py
Outdated
| # project_id must be set if not running on GCP. | ||
| # project_id='my-project-id', | ||
| ) | ||
| except (ValueError, NotImplementedError) as e: |
| googlecloudprofiler.start( | ||
| service='hello-profiler', | ||
| service_version='1.0.1', | ||
| verbose=3, |
There was a problem hiding this comment.
3 is a magic number. Does the client library provide any constants which may be used instead? Otherwise create a constant for the sample.
There was a problem hiding this comment.
I just added a comment. The client code doesn't provide constants for logging levels. This sample will serve as a code snippet of calling the start function. I'm a little hesitate to create a constant, which may confuse the reader.
There was a problem hiding this comment.
Is there a FR to add such constants to the client? I think such constants would be useful.
There was a problem hiding this comment.
Created a FR. Though I'm not sure if we definitely want to do that change, the number is consistent with the profiling agents in other languages.
| googlecloudprofiler.start( | ||
| service='hello-profiler', | ||
| service_version='1.0.1', | ||
| verbose=3, |
There was a problem hiding this comment.
Is there a FR to add such constants to the client? I think such constants would be useful.
Add code samples for https://cloud.google.com/profiler.