Browse Source

Reset mapping file to empty string if it was patched to be the source URL.

When the source is remote and a mapping doesn't have either build ID or
file field set, the file field is set to the source URL so that the
proper source from the source mapping is used during symbolz processing.
Before this change, the file field would continue to point to the URL
producing the URL in pprof output which confuses users.

With the change it resets the file field back to the empty string. It
also now skips the URL-like paths during local symbolization as
reading at that path is not going to succeed.

We discussed switching to generating more unique IDs. On the second
thought, I propose leaving these to be URLs as that seems unique enough
and in case this field leaks into the tool or log output seeing the URL
seems still friendlier than some arbitrarily prefixed string.
Alexey Alexandrov 8 years ago
parent
commit
ddacd6aa78
3 changed files with 85 additions and 0 deletions
  1. 15
    0
      internal/driver/fetch.go
  2. 62
    0
      internal/driver/fetch_test.go
  3. 8
    0
      internal/symbolizer/symbolizer.go

+ 15
- 0
internal/driver/fetch.go View File

72
 		return nil, err
72
 		return nil, err
73
 	}
73
 	}
74
 	p.RemoveUninteresting()
74
 	p.RemoveUninteresting()
75
+	unsourceMappings(p)
75
 
76
 
76
 	// Save a copy of the merged profile if there is at least one remote source.
77
 	// Save a copy of the merged profile if there is at least one remote source.
77
 	if save {
78
 	if save {
283
 			// If there is no build id or source file, use the source as the
284
 			// If there is no build id or source file, use the source as the
284
 			// mapping file. This will enable remote symbolization for this
285
 			// mapping file. This will enable remote symbolization for this
285
 			// mapping, in particular for Go profiles on the legacy format.
286
 			// mapping, in particular for Go profiles on the legacy format.
287
+			// The source is reset back to empty string by unsourceMapping
288
+			// which is called after symbolization is finished.
286
 			m.File = source
289
 			m.File = source
287
 			key = source
290
 			key = source
288
 		}
291
 		}
291
 	return ms
294
 	return ms
292
 }
295
 }
293
 
296
 
297
+// unsourceMappings iterates over the mappings in a profile and replaces file
298
+// set to the remote source URL by collectMappingSources back to empty string.
299
+func unsourceMappings(p *profile.Profile) {
300
+	for _, m := range p.Mapping {
301
+		if m.BuildID == "" {
302
+			if u, err := url.Parse(m.File); err == nil && u.IsAbs() {
303
+				m.File = ""
304
+			}
305
+		}
306
+	}
307
+}
308
+
294
 // locateBinaries searches for binary files listed in the profile and, if found,
309
 // locateBinaries searches for binary files listed in the profile and, if found,
295
 // updates the profile accordingly.
310
 // updates the profile accordingly.
296
 func locateBinaries(p *profile.Profile, s *source, obj plugin.ObjTool, ui plugin.UI) {
311
 func locateBinaries(p *profile.Profile, s *source, obj plugin.ObjTool, ui plugin.UI) {

+ 62
- 0
internal/driver/fetch_test.go View File

21
 	"net/url"
21
 	"net/url"
22
 	"os"
22
 	"os"
23
 	"path/filepath"
23
 	"path/filepath"
24
+	"reflect"
24
 	"regexp"
25
 	"regexp"
25
 	"testing"
26
 	"testing"
26
 	"time"
27
 	"time"
76
 	os.Setenv("PPROF_BINARY_PATH", savePath)
77
 	os.Setenv("PPROF_BINARY_PATH", savePath)
77
 }
78
 }
78
 
79
 
80
+func TestCollectMappingSources(t *testing.T) {
81
+	const startAddress uint64 = 0x40000
82
+	const url = "http://example.com"
83
+	for _, tc := range []struct {
84
+		file, buildID string
85
+		want          plugin.MappingSources
86
+	}{
87
+		{"/usr/bin/binary", "buildId", mappingSources("buildId", url, startAddress)},
88
+		{"/usr/bin/binary", "", mappingSources("/usr/bin/binary", url, startAddress)},
89
+		{"", "", mappingSources(url, url, startAddress)},
90
+	} {
91
+		p := &profile.Profile{
92
+			Mapping: []*profile.Mapping{
93
+				{
94
+					File:    tc.file,
95
+					BuildID: tc.buildID,
96
+					Start:   startAddress,
97
+				},
98
+			},
99
+		}
100
+		got := collectMappingSources(p, url)
101
+		if !reflect.DeepEqual(got, tc.want) {
102
+			t.Errorf("%s:%s, want %s, got %s", tc.file, tc.buildID, tc.want, got)
103
+		}
104
+	}
105
+}
106
+
107
+func TestUnsourceMappings(t *testing.T) {
108
+	for _, tc := range []struct {
109
+		file, buildID, want string
110
+	}{
111
+		{"/usr/bin/binary", "buildId", "/usr/bin/binary"},
112
+		{"http://example.com", "", ""},
113
+	} {
114
+		p := &profile.Profile{
115
+			Mapping: []*profile.Mapping{
116
+				{
117
+					File:    tc.file,
118
+					BuildID: tc.buildID,
119
+				},
120
+			},
121
+		}
122
+		unsourceMappings(p)
123
+		if got := p.Mapping[0].File; got != tc.want {
124
+			t.Errorf("%s:%s, want %s, got %s", tc.file, tc.buildID, tc.want, got)
125
+		}
126
+	}
127
+}
128
+
79
 type testObj struct {
129
 type testObj struct {
80
 	home string
130
 	home string
81
 }
131
 }
125
 	}
175
 	}
126
 }
176
 }
127
 
177
 
178
+// mappingSources creates MappingSources map with a single item.
179
+func mappingSources(key, source string, start uint64) plugin.MappingSources {
180
+	return plugin.MappingSources{
181
+		key: []struct {
182
+			Source string
183
+			Start  uint64
184
+		}{
185
+			{Source: source, Start: start},
186
+		},
187
+	}
188
+}
189
+
128
 // stubHTTPGet intercepts a call to http.Get and rewrites it to use
190
 // stubHTTPGet intercepts a call to http.Get and rewrites it to use
129
 // "file://" to get the profile directly from a file.
191
 // "file://" to get the profile directly from a file.
130
 func stubHTTPGet(source string, _ time.Duration) (*http.Response, error) {
192
 func stubHTTPGet(source string, _ time.Duration) (*http.Response, error) {

+ 8
- 0
internal/symbolizer/symbolizer.go View File

21
 	"fmt"
21
 	"fmt"
22
 	"io/ioutil"
22
 	"io/ioutil"
23
 	"net/http"
23
 	"net/http"
24
+	"net/url"
24
 	"path/filepath"
25
 	"path/filepath"
25
 	"strings"
26
 	"strings"
26
 
27
 
299
 			continue
300
 			continue
300
 		}
301
 		}
301
 
302
 
303
+		// Skip mappings pointing to a source URL
304
+		if m.BuildID == "" {
305
+			if u, err := url.Parse(m.File); err == nil && u.IsAbs() {
306
+				continue
307
+			}
308
+		}
309
+
302
 		f, err := obj.Open(m.File, m.Start, m.Limit, m.Offset)
310
 		f, err := obj.Open(m.File, m.Start, m.Limit, m.Offset)
303
 		if err != nil {
311
 		if err != nil {
304
 			ui.PrintErr("Local symbolization failed for ", name, ": ", err)
312
 			ui.PrintErr("Local symbolization failed for ", name, ": ", err)