Skip to content

Commit 35668a1

Browse files
authored
[cmd/mdatagen]: Improve loading errors for clarity (open-telemetry#13206)
#### Description Expose `go list -f` stderr if the command fails. <!-- Issue number if applicable --> #### Link to tracking issue Fixes open-telemetry#13205 <!--Describe what testing was performed and which tests were added.--> #### Testing Manual, unit tests <!--Describe the documentation added.--> #### Documentation No new docs required
1 parent a215b9f commit 35668a1

File tree

7 files changed

+83
-7
lines changed

7 files changed

+83
-7
lines changed
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: 'enhancement'
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: mdatagen
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: "Taught mdatagen to print the `go list` stderr output on failures, and to run `go list` where the metadata file is."
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [13205]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: [user]

cmd/mdatagen/internal/command_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,8 @@ func TestRunContents(t *testing.T) {
199199
require.NoError(t, err)
200200
metadataFile := filepath.Join(tmpdir, "metadata.yaml")
201201
require.NoError(t, os.WriteFile(metadataFile, ymlContent, 0o600))
202+
require.NoError(t, os.WriteFile(filepath.Join(tmpdir, "empty.go"), []byte("package shortname"), 0o600))
203+
require.NoError(t, os.WriteFile(filepath.Join(tmpdir, "go.mod"), []byte("module shortname"), 0o600))
202204
require.NoError(t, os.WriteFile(filepath.Join(tmpdir, "README.md"), []byte(`
203205
<!-- status autogenerated section -->
204206
foo

cmd/mdatagen/internal/loader.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ package internal // import "go.opentelemetry.io/collector/cmd/mdatagen/internal"
55

66
import (
77
"context"
8+
"errors"
9+
"fmt"
810
"os/exec"
911
"path/filepath"
1012
"strings"
@@ -42,7 +44,7 @@ func LoadMetadata(filePath string) (Metadata, error) {
4244
return md, err
4345
}
4446
if md.ScopeName == "" {
45-
md.ScopeName, err = packageName()
47+
md.ScopeName, err = packageName(filepath.Dir(filePath))
4648
if err != nil {
4749
return md, err
4850
}
@@ -80,11 +82,17 @@ func shortFolderName(filePath string) string {
8082
return parentFolder
8183
}
8284

83-
func packageName() (string, error) {
85+
func packageName(filePath string) (string, error) {
8486
cmd := exec.Command("go", "list", "-f", "{{.ImportPath}}")
87+
cmd.Dir = filePath
8588
output, err := cmd.Output()
8689
if err != nil {
87-
return "", err
90+
var ee *exec.ExitError
91+
if errors.As(err, &ee) {
92+
return "", fmt.Errorf("unable to determine package name: %v failed: (stderr) %v", cmd.Args, string(ee.Stderr))
93+
}
94+
95+
return "", fmt.Errorf("unable to determine package name: %v failed: %v %w", cmd.Args, string(output), err)
8896
}
8997
return strings.TrimSpace(string(output)), nil
9098
}

cmd/mdatagen/internal/loader_test.go

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
package internal
55

66
import (
7+
"os"
8+
"path/filepath"
79
"testing"
810

911
"github.com/stretchr/testify/require"
@@ -13,6 +15,22 @@ import (
1315
"go.opentelemetry.io/collector/pdata/pmetric"
1416
)
1517

18+
func TestTwoPackagesInDirectory(t *testing.T) {
19+
contents, err := os.ReadFile("testdata/twopackages.yaml")
20+
require.NoError(t, err)
21+
tempDir := t.TempDir()
22+
metadataPath := filepath.Join(tempDir, "metadata.yaml")
23+
// we create a trivial module and packages to avoid having invalid go checked into our test directory.
24+
require.NoError(t, os.WriteFile(filepath.Join(tempDir, "go.mod"), []byte("module twopackages"), 0o600))
25+
require.NoError(t, os.WriteFile(filepath.Join(tempDir, "package1.go"), []byte("package package1"), 0o600))
26+
require.NoError(t, os.WriteFile(filepath.Join(tempDir, "package2.go"), []byte("package package2"), 0o600))
27+
require.NoError(t, os.WriteFile(metadataPath, contents, 0o600))
28+
29+
_, err = LoadMetadata(metadataPath)
30+
require.Error(t, err)
31+
require.ErrorContains(t, err, "unable to determine package name: [go list -f {{.ImportPath}}] failed: (stderr) found packages package1 (package1.go) and package2 (package2.go)")
32+
}
33+
1634
func TestLoadMetadata(t *testing.T) {
1735
tests := []struct {
1836
name string
@@ -348,7 +366,7 @@ func TestLoadMetadata(t *testing.T) {
348366
Type: "subcomponent",
349367
Parent: "parentComponent",
350368
GeneratedPackageName: "metadata",
351-
ScopeName: "go.opentelemetry.io/collector/cmd/mdatagen/internal",
369+
ScopeName: "go.opentelemetry.io/collector/cmd/mdatagen/internal/testdata",
352370
ShortFolderName: "testdata",
353371
Tests: Tests{Host: "componenttest.NewNopHost()"},
354372
},
@@ -358,7 +376,7 @@ func TestLoadMetadata(t *testing.T) {
358376
want: Metadata{
359377
Type: "custom",
360378
GeneratedPackageName: "customname",
361-
ScopeName: "go.opentelemetry.io/collector/cmd/mdatagen/internal",
379+
ScopeName: "go.opentelemetry.io/collector/cmd/mdatagen/internal/testdata",
362380
ShortFolderName: "testdata",
363381
Tests: Tests{Host: "componenttest.NewNopHost()"},
364382
Status: &Status{
@@ -376,7 +394,7 @@ func TestLoadMetadata(t *testing.T) {
376394
want: Metadata{
377395
Type: "test",
378396
GeneratedPackageName: "metadata",
379-
ScopeName: "go.opentelemetry.io/collector/cmd/mdatagen/internal",
397+
ScopeName: "go.opentelemetry.io/collector/cmd/mdatagen/internal/testdata",
380398
ShortFolderName: "testdata",
381399
Tests: Tests{Host: "componenttest.NewNopHost()"},
382400
Status: &Status{
@@ -422,13 +440,17 @@ func TestLoadMetadata(t *testing.T) {
422440
want: Metadata{},
423441
wantErr: "decoding failed due to the following error(s):\n\nerror decoding 'attributes[used_attr].type': invalid type: \"invalidtype\"",
424442
},
443+
{
444+
name: "testdata/~~this file doesn't exist~~.yaml",
445+
wantErr: "unable to read the file file:testdata/~~this file doesn't exist~~.yaml",
446+
},
425447
}
426448
for _, tt := range tests {
427449
t.Run(tt.name, func(t *testing.T) {
428450
got, err := LoadMetadata(tt.name)
429451
if tt.wantErr != "" {
430452
require.Error(t, err)
431-
require.EqualError(t, err, tt.wantErr)
453+
require.ErrorContains(t, err, tt.wantErr)
432454
} else {
433455
require.NoError(t, err)
434456
require.Equal(t, tt.want, got)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package testdata
5+
6+
// this file allows `go list -f` to run in tests and get the scope name.
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package events
5+
6+
// this file allows `go list -f` to run in tests and get the scope name.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
type: sample
2+
github_project: open-telemetry/opentelemetry-collector
3+
4+
status:
5+
class: receiver
6+
stability:
7+
beta: [traces]

0 commit comments

Comments
 (0)