-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
cmd/pprof: If the file under the current path contains a colon, go tool pprof will parse it into a url and will not work properly. #63924
Comments
Change https://go.dev/cl/539595 mentions this issue: |
To fix this problem, we also need to update google/pprof. I have already raised an issue google/pprof#816 |
I don't see which specific Have you tried prefixing the file with Here's how it works (and fails) on my machine. First creating a small demo profile:
Then triggering what I think is the behavior you're describing:
Then working around that by adding a
|
As @rhysh pointed out, it may be hard to for the tool to distinguish a file named something like |
I think from usage habits, when the target source is a file in the current directory, many developers are likely to omit ./ The current processing method I modified is that if the file contains a colon and will be processed by url.Parse, check whether a file with the same name exists before requesting. If there is no localhost:8080 file locally, it will be treated as a URL normally. But if a file named localhost:8080 does exist, I think the user is more likely to want to handle it as a file.
Thank you for taking the time to test and reply. But I think from usage habits, when the target source is a file in the current directory, many developers are likely to omit ./ |
Thank you very much for your reply. I think from usage habits, when the target source is a file in the current directory, many developers are likely to omit ./ The current processing method I modified is that if the file contains a colon and will be processed by url.Parse, check whether a file with the same name exists before requesting. If there is no localhost:8080 file locally, it will be treated as a URL normally. But if a file named localhost:8080 does exist, I think the user is more likely to want to handle it as a file. (Of course, in most cases the file name is unlikely to be localhost:8080, but like mem_2023-11-01_01:01:00. If this type is processed as a URL, it is best to avoid it) Since google/pprof also requires almost the same changes, I am also paying attention to the suggestions of the maintainers of this project. In addition, the two fetch methods in go and google/pprof have almost the same logical judgment, which seems to be able to streamline some code. |
Merged |
Relevant changes have been merged into google/pprof. According to communication and suggestions with the pprof maintainer, if src is a file that exists in the current directory, it will be treated as a file (even if it contains a colon). For details please refer to google/pprof#817 I think it's time to move on to the next step. But I feel that this CL will change the relevant logic of pprof to a certain extent (although it is very small). Should I submit a proposal and request the Go team for review? Thanks. |
Change https://go.dev/cl/547236 mentions this issue: |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
I helped a colleague write a sample code to obtain the pprof file when the memory exceeds the threshold.
I tested it locally and everything was fine. He made certain modifications and then released it to the production environment. When the memory surge occurred, this program obtained the pprof file as expected, but what was confusing and surprising was that when using go tool pprof thefile, an error was reported:
I spent a lot of time trying to research, and one of his modifications was to change the timestamp in the file path to time.Now().Format("2006-01-02_15:04:05") in order to be more intuitive.
That is
filepath := "/xxx/yyy/path/mem" + "." + strconv.Itoa(int(time.Now().Unix())) ---> filepath := "/xxx/yyy/path /mem" + "." + time.Now().Format("2006-01-02_15:04:05")
After adding debug information several times, it was found that it was the adjustURL method in
internal/driver/fetch.go
. Because the file name contained a colon, url.Parse parsed the file path into a url. . .This is confusing and counter-intuitive. After many experiments, I think we can consider determining whether there is a file with the same name in the directory. If so, treat it as a file instead of a URL.
(We only need to consider the case of executing go tool pprof in the directory where the profile is located. In the case of go tool pprof a/b/mem_pprof_file_2006_01_02_15:04:05, it will not be recognized as a URL now)
What did you expect to see?
I want to be able to use pprof normally without renaming the file
What did you see instead?
The text was updated successfully, but these errors were encountered: