Recognize https+insecure scheme for symbolization POSTs. #111
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added a test, verified the test failed before the fix. This fixes google#94.
|
Ping, we'd love to have this for CockroachDB. |
rauls5382
approved these changes
Mar 14, 2017
tamird
reviewed
Mar 14, 2017
| if err != nil { | ||
| t.Fatalf("ReadFile(%s) got error %v, want no error", outputTempFile.Name(), err) | ||
| } | ||
| if got, want := string(b), "TestHttpsInsecure"; !strings.Contains(got, want) { |
There was a problem hiding this comment.
this test doesn't pass on 1.8:
--- FAIL: TestHttpsInsecure (5.03s)
driver_test.go:1098: Pprof(&{{} 0xc4200157c0 <nil> 0xc420167f40 0xc42016a960 0xc42000e048}): got Type: cpu
Time: Mar 14, 2017 at 6:39pm (EDT)
Duration: 5s, Total samples = 4.36s (87.13%)
Showing nodes accounting for 4.36s, 100% of 4.36s total
flat flat% sum% cum cum%
4.36s 100% 100% 4.36s 100% <unknown>
, want "TestHttpsInsecure" substring
FAIL
It does pass on Go 1.7, and the profile above just looks garbled, so I'm not sure what to make of it.
There was a problem hiding this comment.
Reverted, looking.
There was a problem hiding this comment.
Which platform are you running on? I checked with 1.7 and 1.8 and the test passes for me.
@aalexand0:pprof$ ( export PATH=~/sw/go1.7/bin:$PATH && export GOROOT=~/sw/go1.7 && go test -v ./internal/driver/... ) 2>&1 | grep Insecure
=== RUN TestHttpsInsecure
--- PASS: TestHttpsInsecure (5.03s)
@aalexand0:pprof$ ( export PATH=~/sw/go1.8/bin:$PATH && export GOROOT=~/sw/go1.8 && go test -v ./internal/driver/... ) 2>&1 | grep Insecure
=== RUN TestHttpsInsecure
--- PASS: TestHttpsInsecure (5.03s)
There was a problem hiding this comment.
@tamird the previous comment was a question to you, for clarity
There was a problem hiding this comment.
$ go17
$ go version
go version go1.7.5 darwin/amd64
$ go test ./internal/driver -run TestHttpsInsecure -v
=== RUN TestHttpsInsecure
Fetching profile over HTTP from https+insecure://127.0.0.1:51202/debug/pprof/profile?seconds=5
Saved profile in /Users/tamird/pprof/pprof.samples.cpu.009.pb.gz
Generating report in /var/folders/lf/1_qqs3815tzgkhtrbrjs913m0000gn/T/profile_output830636954
--- PASS: TestHttpsInsecure (5.03s)
PASS
ok github.com/google/pprof/internal/driver 5.037s
$ go18
$ go version
go version go1.8 darwin/amd64
$ go test ./internal/driver -run TestHttpsInsecure -v
=== RUN TestHttpsInsecure
Fetching profile over HTTP from https+insecure://127.0.0.1:51211/debug/pprof/profile?seconds=5
Saved profile in /Users/tamird/pprof/pprof.samples.cpu.010.pb.gz
Generating report in /var/folders/lf/1_qqs3815tzgkhtrbrjs913m0000gn/T/profile_output478111872
--- FAIL: TestHttpsInsecure (5.03s)
driver_test.go:1100: Pprof(&{{} 0xc420220540 <nil> 0xc4202263a0 0xc420182840 0xc42008a1d0}): got Type: cpu
Time: Mar 14, 2017 at 7:33pm (EDT)
Duration: 5.01s, Total samples = 4.17s (83.30%)
Showing nodes accounting for 4.17s, 100% of 4.17s total
flat flat% sum% cum cum%
4.17s 100% 100% 4.17s 100% <unknown>
, want "TestHttpsInsecure" substring
FAIL
exit status 1
FAIL github.com/google/pprof/internal/driver 5.043s
There was a problem hiding this comment.
This also fails for me in Linux (in docker):
$ go version
go version go1.8 linux/amd64
$ go test ./internal/driver -run TestHttpsInsecure -v
=== RUN TestHttpsInsecure
Fetching profile over HTTP from https+insecure://127.0.0.1:43243/debug/pprof/profile?seconds=5
Saved profile in /root/pprof/pprof.driver.test.samples.cpu.001.pb.gz
Generating report in /go/src/github.com/cockroachdb/cockroach/artifacts/profile_output695402950
--- FAIL: TestHttpsInsecure (5.07s)
driver_test.go:1100: Pprof(&{{} 0xc4201ec780 <nil> 0xc4201f6820 0xc42017a780 0xc42000e210}): got File: driver.test
Type: cpu
Time: Mar 14, 2017 at 11:35pm (UTC)
Duration: 5s, Total samples = 4.96s (99.15%)
Showing nodes accounting for 4.95s, 99.80% of 4.96s total
Dropped 3 nodes (cum <= 0.02s)
flat flat% sum% cum cum%
4.95s 99.80% 99.80% 4.95s 99.80% runtime._ExternalCode
0 0% 99.80% 4.95s 99.80% runtime._System
, want "TestHttpsInsecure" substring
FAIL
exit status 1
FAIL github.com/google/pprof/internal/driver 5.122sThere was a problem hiding this comment.
I could reproduce it on my Mac so will be looking, thanks!
This was referenced Mar 14, 2017
aalexand
added a commit
to aalexand/pprof
that referenced
this pull request
Mar 15, 2017
This reverts the revert of google#111 relaxing the test to make it pass as the issue reported in google#111 is caused by unrelated google#120. Added a test, verified the test failed before the fix. This fixes google#94.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added a test, verified the test failed before the fix.
This fixes #94.