Glic: Add metrics and traces for webui initialization [chromium/src : main]
Aldo Culquicondor (Gerrit)
Aldo Culquicondor voted and added 2 comments![]( "Open in Gerrit")
Votes added by Aldo Culquicondor
| Commit-Queue | +1 |
2 comments
Aldo Culquicondorhistograms.xml
- LGTM
CL Description
From the time of the Glic instance creation until entering the
kBeginLoad state.This seems like an incomplete sentence? Should it start with "This CL adds metrics capturing the duration from the time..."
Even better would be something like:
```
Glic: Add metric for webui initialization.This CL adds the following metrics:
- Foo - Captures the time, in units, it takes to Foo, from creating to commit.
- Bar - Captures the time, in units, it takes to Bar, from load starts, to commit.
- ```
Sorry, that was the old CL description. Please see the update.
However, I don't see the need to list every histogram in the description, when they are properly documented in code.
Related details
- Etienne Pierre-Doray
- Roger McFarlane
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Roger McFarlane (Gerrit)
Roger McFarlane voted and added 1 comment![]( "Open in Gerrit")
Votes added by Roger McFarlane
| Code-Review | +1 |
Aldo Culquicondorhistograms.xml
- LGTM
CL Description
From the time of the Glic instance creation until entering the
kBeginLoad state.This seems like an incomplete sentence? Should it start with "This CL adds metrics capturing the duration from the time..."
Even better would be something like:
```
Glic: Add metric for webui initialization.This CL adds the following metrics:
- Foo - Captures the time, in units, it takes to Foo, from creating to commit.
- Bar - Captures the time, in units, it takes to Bar, from load starts, to commit.
- ```
Sorry, that was the old CL description. Please see the update.
However, I don't see the need to list every histogram in the description, when they are properly documented in code.
From the time of the web contents creation until the page load
completes.
Starting the sentence with `From` results in a sentence which is not well-formed.
Replace `From` with `Log` or `Capture` and optionally replace `of` with `from`.
Related details
- Aldo Culquicondor
- Etienne Pierre-Doray
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aldo Culquicondor (Gerrit)
Aldo Culquicondor added 1 comment![]( "Open in Gerrit")
Aldo Culquicondorhistograms.xml
- LGTM
CL Description
From the time of the Glic instance creation until entering the
kBeginLoad state.This seems like an incomplete sentence? Should it start with "This CL adds metrics capturing the duration from the time..."
Even better would be something like:
```
Glic: Add metric for webui initialization.This CL adds the following metrics:
- Foo - Captures the time, in units, it takes to Foo, from creating to commit.
- Bar - Captures the time, in units, it takes to Bar, from load starts, to commit.
- ```
Roger McFarlaneSorry, that was the old CL description. Please see the update.
However, I don't see the need to list every histogram in the description, when they are properly documented in code.
From the time of the web contents creation until the page load
completes.Starting the sentence with `From` results in a sentence which is not well-formed.
Replace `From` with `Log` or `Capture` and optionally replace `of` with `from`.
- Etienne Pierre-Doray
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Etienne Pierre-Doray added 1 comment![]( "Open in Gerrit")
perfetto::Category("glic"),Could you add a short description of what events this includes? (and what glic stands for)
Related details
- Aldo Culquicondor
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aldo Culquicondor (Gerrit)
Aldo Culquicondor added 1 comment![]( "Open in Gerrit")
Could you add a short description of what events this includes? (and what glic stands for)
It felt a bit redudant to me, as we have glic folders, but I guess not everyone knows what glic is.
Related details
- Etienne Pierre-Doray
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aldo Culquicondor (Gerrit)
Aldo Culquicondor voted Auto-Submit+1![]( "Open in Gerrit")
| Auto-Submit | +1 |
Etienne Pierre-Doray (Gerrit)
Etienne Pierre-Doray voted and added 1 comment![]( "Open in Gerrit")
Votes added by Etienne Pierre-Doray
| Code-Review | +1 |
1 comment
Related details
- Aldo Culquicondor
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aldo Culquicondor (Gerrit)
Aldo Culquicondor voted Commit-Queue+2![]( "Open in Gerrit")
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Glic: Add metrics and traces for webui initialization
Track the time time from the web contents creation until the navigation
commits and from then until the page load completes.
Also added a dedicated glic category.
- M base/trace_event/builtin_categories.h
- M chrome/browser/glic/host/glic_page_handler.cc
- M chrome/browser/glic/host/webui_contents_container.cc
- M chrome/browser/glic/host/webui_contents_container.h
- M chrome/browser/glic/service/glic_instance_impl.cc
- M chrome/browser/glic/service/metrics/glic_instance_metrics.cc
- M tools/metrics/histograms/metadata/glic/histograms.xml
Code-Review: +1 by Roger McFarlane, +1 by Etienne Pierre-Doray, +1 by Dan Harrington
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |