Skip to content

Commit 98f555d

Browse files
authored
[cmd/builder] Use relative paths in Go module replacements by default (#15098)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description When users want to track their generated OCB directory in source control, absolute paths don't work because the `go replace` statements need to resolve on any machine. This PR adds the `dist::use_relative_replace_paths` feature so that `path` values in modules are not forcibly resolved to absolute paths and instead calculated as relative paths to the output directory. <!-- Issue number if applicable --> #### Link to tracking issue Fixes #15097 <!--Describe what testing was performed and which tests were added.--> #### Testing I installed the tool locally and used it in our setup. When I had this module: ```yaml exporters: - gomod: github.com/GoogleCloudPlatform/opentelemetry-operations-collector/components/google-built-opentelemetry-collector/exporter/googleservicecontrolexporter v0.148.0 path: ../components/google-built-opentelemetry-collector/exporter/googleservicecontrolexporter ``` I got the following replacement: ``` replace github.com/GoogleCloudPlatform/opentelemetry-operations-collector/components/google-built-opentelemetry-collector/exporter/googleservicecontrolexporter v0.148.0 => ../../components/google-built-opentelemetry-collector/exporter/googleservicecontrolexporter ``` I was able to build successfully. <!--Describe the documentation added.--> #### Documentation Added to the config docs in README. #### AI Usage Disclosure The code changes in this PR were created with AI assistance via Gemini/Antigravity. <!--Please delete paragraphs that you did not use before submitting.-->
1 parent 5d77770 commit 98f555d

5 files changed

Lines changed: 119 additions & 29 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
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: breaking
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. receiver/otlp)
7+
component: cmd/builder
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: In the generated Collector source, the `replace` statements in the Go module will now use relative paths by default.
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [15097]
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+
We expect that this will not break existing use-cases where the generated collector is only used in an interim manner for builds.
20+
It enables the possibility of tracking the generated Collector code as a longer living artifact, allowing it to be run on any
21+
machine (whereas absolute paths will be different depending on the machine the Collector source is generated on.)
22+
We have added `dist::use_absolute_replace_paths` to go back to the absolute path behaviour in the case where there is an
23+
unforeseen use-case that requires absolute paths.
24+
25+
# Optional: The change log or logs in which this entry should be included.
26+
# e.g. '[user]' or '[user, api]'
27+
# Include 'user' if the change is relevant to end users.
28+
# Include 'api' if there is a change to a library API.
29+
# Default: '[user]'
30+
change_logs: []

cmd/builder/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ dist:
160160
version: "1.0.0" # the version for your custom OpenTelemetry Collector. Optional.
161161
go: "/usr/bin/go" # which Go binary to use to compile the generated sources. Optional.
162162
debug_compilation: false # enabling this causes the builder to keep the debug symbols in the resulting binary. Optional.
163+
use_absolute_replace_paths: false # when using local path overrides, uses absolute paths instead of relative paths. Optional.
163164
exporters:
164165
- gomod: "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/alibabacloudlogserviceexporter v0.146.0" # the Go module for the component. Required.
165166
import: "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/alibabacloudlogserviceexporter" # the import path for the component. Optional.

cmd/builder/internal/builder/config.go

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,16 @@ type ConfResolver struct {
6767

6868
// Distribution holds the parameters for the final binary
6969
type Distribution struct {
70-
Module string `mapstructure:"module,omitempty"`
71-
Name string `mapstructure:"name"`
72-
Go string `mapstructure:"go,omitempty"`
73-
Description string `mapstructure:"description"`
74-
OutputPath string `mapstructure:"output_path"`
75-
Version string `mapstructure:"version,omitempty"`
76-
BuildTags string `mapstructure:"build_tags,omitempty"`
77-
DebugCompilation bool `mapstructure:"debug_compilation,omitempty"`
78-
CGoEnabled bool `mapstructure:"cgo_enabled,omitempty"`
70+
Module string `mapstructure:"module,omitempty"`
71+
Name string `mapstructure:"name"`
72+
Go string `mapstructure:"go,omitempty"`
73+
Description string `mapstructure:"description"`
74+
OutputPath string `mapstructure:"output_path"`
75+
Version string `mapstructure:"version,omitempty"`
76+
BuildTags string `mapstructure:"build_tags,omitempty"`
77+
DebugCompilation bool `mapstructure:"debug_compilation,omitempty"`
78+
CGoEnabled bool `mapstructure:"cgo_enabled,omitempty"`
79+
UseAbsoluteReplacePaths bool `mapstructure:"use_absolute_replace_paths,omitempty"`
7980
}
8081

8182
// Module represents a receiver, exporter, processor or extension for the distribution
@@ -171,42 +172,42 @@ func (c *Config) ParseModules() error {
171172
var err error
172173
usedNames := make(map[string]int)
173174

174-
c.Extensions, err = parseModules(c.Extensions, usedNames)
175+
c.Extensions, err = c.parseModules(c.Extensions, usedNames)
175176
if err != nil {
176177
return err
177178
}
178179

179-
c.Receivers, err = parseModules(c.Receivers, usedNames)
180+
c.Receivers, err = c.parseModules(c.Receivers, usedNames)
180181
if err != nil {
181182
return err
182183
}
183184

184-
c.Exporters, err = parseModules(c.Exporters, usedNames)
185+
c.Exporters, err = c.parseModules(c.Exporters, usedNames)
185186
if err != nil {
186187
return err
187188
}
188189

189-
c.Processors, err = parseModules(c.Processors, usedNames)
190+
c.Processors, err = c.parseModules(c.Processors, usedNames)
190191
if err != nil {
191192
return err
192193
}
193194

194-
c.Connectors, err = parseModules(c.Connectors, usedNames)
195+
c.Connectors, err = c.parseModules(c.Connectors, usedNames)
195196
if err != nil {
196197
return err
197198
}
198199

199-
telemetry, err := parseModules([]Module{c.Telemetry}, usedNames)
200+
telemetry, err := c.parseModules([]Module{c.Telemetry}, usedNames)
200201
if err != nil {
201202
return err
202203
}
203204
c.Telemetry = telemetry[0]
204205

205-
c.ConfmapProviders, err = parseModules(c.ConfmapProviders, usedNames)
206+
c.ConfmapProviders, err = c.parseModules(c.ConfmapProviders, usedNames)
206207
if err != nil {
207208
return err
208209
}
209-
c.ConfmapConverters, err = parseModules(c.ConfmapConverters, usedNames)
210+
c.ConfmapConverters, err = c.parseModules(c.ConfmapConverters, usedNames)
210211
if err != nil {
211212
return err
212213
}
@@ -245,7 +246,7 @@ func validateTelemetry(c *Config) error {
245246
return nil
246247
}
247248

248-
func parseModules(mods []Module, usedNames map[string]int) ([]Module, error) {
249+
func (c *Config) parseModules(mods []Module, usedNames map[string]int) ([]Module, error) {
249250
var parsedModules []Module
250251
for _, mod := range mods {
251252
if mod.Import == "" {
@@ -275,13 +276,28 @@ func parseModules(mods []Module, usedNames map[string]int) ([]Module, error) {
275276
// Check if path is empty, otherwise filepath.Abs replaces it with current path ".".
276277
if mod.Path != "" {
277278
var err error
278-
mod.Path, err = filepath.Abs(mod.Path)
279+
absPath, err := filepath.Abs(mod.Path)
279280
if err != nil {
280-
return mods, fmt.Errorf("module has a relative \"path\" element, but we couldn't resolve the current working dir: %w", err)
281+
return mods, fmt.Errorf("failed to resolve absolute path for %s: %w", mod.Path, err)
281282
}
282-
// Check if the path exists
283-
if _, err := os.Stat(mod.Path); os.IsNotExist(err) {
284-
return mods, fmt.Errorf("filepath does not exist: %s", mod.Path)
283+
284+
if c.Distribution.UseAbsoluteReplacePaths {
285+
mod.Path = absPath
286+
} else {
287+
absOutputPath, err := filepath.Abs(c.Distribution.OutputPath)
288+
if err != nil {
289+
return mods, fmt.Errorf("failed to resolve absolute path for output dir %s: %w", c.Distribution.OutputPath, err)
290+
}
291+
mod.Path, err = filepath.Rel(absOutputPath, absPath)
292+
if err != nil {
293+
return mods, fmt.Errorf("failed to make path relative to output dir: %w", err)
294+
}
295+
}
296+
mod.Path = filepath.ToSlash(mod.Path)
297+
298+
// Check if the path exists using the absolute path
299+
if _, err := os.Stat(absPath); os.IsNotExist(err) {
300+
return mods, fmt.Errorf("filepath does not exist: %s", absPath)
285301
}
286302
}
287303

cmd/builder/internal/builder/config_test.go

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package builder
55

66
import (
77
"os"
8+
"path/filepath"
89
"reflect"
910
"strings"
1011
"testing"
@@ -171,9 +172,12 @@ func TestInvalidConverter(t *testing.T) {
171172
require.Error(t, err, "expected an error when parsing invalid modules")
172173
}
173174

174-
func TestRelativePath(t *testing.T) {
175+
func TestAbsoluteReplacePaths(t *testing.T) {
175176
// prepare
176177
cfg := Config{
178+
Distribution: Distribution{
179+
UseAbsoluteReplacePaths: true,
180+
},
177181
Extensions: []Module{{
178182
GoMod: "some-module",
179183
Path: "./templates",
@@ -187,7 +191,8 @@ func TestRelativePath(t *testing.T) {
187191
// verify
188192
cwd, err := os.Getwd()
189193
require.NoError(t, err)
190-
assert.True(t, strings.HasPrefix(cfg.Extensions[0].Path, cwd))
194+
normalizedCwd := filepath.ToSlash(cwd)
195+
assert.True(t, strings.HasPrefix(cfg.Extensions[0].Path, normalizedCwd), "expected path %q to have prefix %q", cfg.Extensions[0].Path, normalizedCwd)
191196
}
192197

193198
func TestModuleFromCore(t *testing.T) {
@@ -397,6 +402,44 @@ func TestSkipsNilFieldValidation(t *testing.T) {
397402
assert.NoError(t, cfg.Validate())
398403
}
399404

405+
func TestParseModulesDefaultRelativePath(t *testing.T) {
406+
cfg := Config{
407+
Distribution: Distribution{
408+
OutputPath: "./output",
409+
},
410+
Extensions: []Module{{
411+
GoMod: "some-module",
412+
Path: "./templates",
413+
}},
414+
}
415+
416+
err := cfg.ParseModules()
417+
require.NoError(t, err)
418+
419+
assert.Equal(t, "../templates", cfg.Extensions[0].Path)
420+
}
421+
422+
func TestParseModulesAbsoluteReplacePathsFlag(t *testing.T) {
423+
cfg := Config{
424+
Distribution: Distribution{
425+
OutputPath: "./output",
426+
UseAbsoluteReplacePaths: true,
427+
},
428+
Extensions: []Module{{
429+
GoMod: "some-module",
430+
Path: "./templates",
431+
}},
432+
}
433+
434+
err := cfg.ParseModules()
435+
require.NoError(t, err)
436+
437+
cwd, err := os.Getwd()
438+
require.NoError(t, err)
439+
expectedPath := filepath.ToSlash(filepath.Join(cwd, "templates"))
440+
assert.Equal(t, expectedPath, cfg.Extensions[0].Path)
441+
}
442+
400443
func TestIsEmpty(t *testing.T) {
401444
for _, tt := range []struct {
402445
name string

cmd/builder/internal/builder/main_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ func TestReplaceStatementsAreComplete(t *testing.T) {
418418
// This ensures the resulting go.mod file has maximum coverage of modules
419419
// that exist in the Core repository.
420420
usedNames := make(map[string]int)
421-
cfg.Exporters, err = parseModules([]Module{
421+
cfg.Exporters, err = cfg.parseModules([]Module{
422422
{
423423
GoMod: "go.opentelemetry.io/collector/exporter/debugexporter v1.9999.9999",
424424
},
@@ -433,7 +433,7 @@ func TestReplaceStatementsAreComplete(t *testing.T) {
433433
},
434434
}, usedNames)
435435
require.NoError(t, err)
436-
cfg.Receivers, err = parseModules([]Module{
436+
cfg.Receivers, err = cfg.parseModules([]Module{
437437
{
438438
GoMod: "go.opentelemetry.io/collector/receiver/nopreceiver v1.9999.9999",
439439
},
@@ -442,13 +442,13 @@ func TestReplaceStatementsAreComplete(t *testing.T) {
442442
},
443443
}, usedNames)
444444
require.NoError(t, err)
445-
cfg.Extensions, err = parseModules([]Module{
445+
cfg.Extensions, err = cfg.parseModules([]Module{
446446
{
447447
GoMod: "go.opentelemetry.io/collector/extension/zpagesextension v1.9999.9999",
448448
},
449449
}, usedNames)
450450
require.NoError(t, err)
451-
cfg.Processors, err = parseModules([]Module{
451+
cfg.Processors, err = cfg.parseModules([]Module{
452452
{
453453
GoMod: "go.opentelemetry.io/collector/processor/batchprocessor v1.9999.9999",
454454
},

0 commit comments

Comments
 (0)