Distrogen update #372

avilevy18 · 2025-07-22T17:03:52Z

Updating distrogen to add full project generation. It assumes that there will always be one distribution per project.

…_update

cmd/distrogen/distribution.go

cmd/distrogen/component.go

cmd/distrogen/distrogen.go

cmd/distrogen/project.go

cmd/distrogen/templates/project/Makefile

…_update

braydonk

I ran a manual test and most things I tried seemed to work. I encountered one bug, which is that both a distrogen and .distrogen folder were generated.

I think it would be a good idea to put together project generation golden test cases, similar to what we have in testdata/generator, which are defined in distribution_test.go. You could also add full project generation test cases to check those goldens. There is certainly more we could do for testing, but I think at least having that would be a good start (and would catch the bug I encountered).

After that, the scratch folder can be removed and we can formally move this to in review.

braydonk · 2025-07-30T13:28:04Z

cmd/distrogen/distribution.go

 	}

+	for _, tmpl := range templates {
+		logger.Debug("context of template", "ctx", tmpl.Context)


We can probably get rid of these debug logs.

cmd/distrogen/registry.go

jefferbrecht · 2025-08-29T15:36:13Z

Please add a motivation to the PR description. What problem is this solving? It doesn't look like a trivial change and the uninformed reader is not going to understand what's happening here.

ridwanmsharif

Please add a motivation to the PR description. What problem is this solving? It doesn't look like a trivial change and the uninformed reader is not going to understand what's happening here.

+1 to Jeffs comment, I got pretty lost in this deep dark forest so consider this a partial review for now

cmd/distrogen/components.go

ridwanmsharif · 2025-09-04T21:25:33Z

cmd/distrogen/registry.go


 var ErrComponentNotFound = errors.New("component not found")

+type ComponentType string


[nit] Why does this have to be of type string and not an enum?

It's more convenient to use it as a key to the registry when we're editing it based on the component type we're trying to add.

ridwanmsharif · 2025-09-04T21:27:02Z

cmd/distrogen/registry.go

+type ComponentType string
+
+const (
+	Receiver  ComponentType = "receiver"


Interesting that we are adding support for all these types. Receivers I do get, but do we have any examples of why we think the others are useful too?

I'm trying to convince myself of a use case where we would want to do this? Is this motivated by the processors that the Ops Agent uses?

Most of this PR was designed for the general use case, not just for us. There's no logical reason to lock down what types of custom components are allowed in a registry that I can think of.

ridwanmsharif · 2025-09-04T21:29:35Z

cmd/distrogen/project.go

+)
+
+type ProjectGenerator struct {
+	Spec       *DistributionSpec


Why are all these exported? They seem to only be used within this package?

I have been keeping the door open for restructuring the project; right now it's still a flat folder but it's possible it might become a cmd folder and then a separate package for the whole distrogen API. I haven't had any time to make a sweeping change like that but don't want to close the door on it either (it would be more annoying if these were all unexported and then had to all be renamed when it gets moved).

ridwanmsharif · 2025-09-04T21:31:57Z

cmd/distrogen/project.go

+
+type ProjectGenerator struct {
+	Spec       *DistributionSpec
+	FileMode   fs.FileMode


When is this set to something other than 0777? If we don't set it anything else, we don't need to be part of this struct, or part of the Get.*TemplateSet signatures.

There was a bit of dev history with this one. When developing on a system with a particular umask set (which it actually is on our internal development machines) the files could be generated with permissions that caused building in Dockerfiles to not work. As a result I had to add the filemode to any os API calls. At first I used a global default variable for that, and opted to make it part of the structs so that if stuff got split up we wouldn't have to add it later.

We could make a choice that we don't need to ever change it and always refer to the global default. At the time I was messing with a number of different values but gave up and ended on 0777 just to make it work and have since forgotten about it.

ridwanmsharif · 2025-09-04T21:45:13Z

cmd/distrogen/project.go

+	if err := os.MkdirAll(filepath.Join(generatePath, "templates"), pg.FileMode); err != nil {
+		dirErrors = append(dirErrors, err)
+	}
+	if _, err := os.Create(filepath.Join(generatePath, "templates", EMPTY_FILE_NAME)); err != nil {


I'm wondering why we create this file here manually but below we rely on the templates to create .distrogen/VERSION

I had an issue with go:embed where it couldn't find nested hidden files so it couldn't find the .distrogen folder. Now that I have fixed that will multiple go:embed this is no longer needed. I removed the manual creation of the .distrogen folder.

cmd/distrogen/main.go

cmd/distrogen/components.go

ridwanmsharif · 2025-09-04T21:55:23Z

cmd/distrogen/component.go

+		Name:     componentName,
+	}
+
+	switch componentType {


making this an enum like I suggested above should get rid of this switch statement with all fallthroughs

If it was an enum we'd just move the switch case from here to where we use it to index the registry instead, idk which one is better seems like the same level of complexity

cmd/distrogen/component.go

cmd/distrogen/testdata/generator/project/golden/spec.yaml

ridwanmsharif · 2025-09-05T03:52:51Z

cmd/distrogen/templates/project/go.mod.go.tmpl

@@ -0,0 +1,4 @@
+module {{ .Spec.Module }}
+
+go {{ .Spec.RenderGoMajorVersion }}


Feels weird that we let people specify the go version but then only use the major version here.

I think typically in the go.mod file you can use the patch version too. And failing that you can also use the toolchain of the patch version you want. Like https://github.com/jaegertracing/jaeger/blob/fa9a5e7d31fcc1163b6bbed741dc288146871959/go.mod#L3-L5

Not saying we need to do this, but was wondering why we do what we do now

From my research go.mod for the collector don't include the patch version

https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/go.mod
https://github.com/open-telemetry/opentelemetry-collector/blob/main/go.mod

Whether you include the patch or not is orthogonal to whether it's a collector or not. Sometimes in Go you need to specify a patch version if your code relies on some quirk of the language/toolchain that's only available after a certain patch level.

You can choose to outright disallow patch levels if you really think it's a pitfall, but if that's the case, do it loudly by raising an error instead of silently truncating it.

(Unfortunate that Go itself doesn't follow semver here... I would've said that RenderGoMajorVersion should actually be named RenderGoMajorAndMinorVersion, but no, that's actually what Go refers to as a major version...)

This is a convention of the project (and in fact we got ourselves into trouble in the Collector for specifying the patch version of Go in our operations-go modules). If you specify the version it makes Go's automatic toolchain resolution do some automatic downloading of the version that some random module asked for. It might be fine for our case but I was just thinking we should follow the convention of upstream on this.

…der.

…no longer necessary

jefferbrecht · 2025-09-08T13:06:55Z

cmd/distrogen/registry.go

(doesn't need to block this PR, this is more of a general wondering)

Are there any docs explaining the need for a component registry?

If I'm wearing my component author hat, what kind of indexing does the registry provide that the built-in module/package management system in Go doesn't provide already? I'm already able to broadcast my component to the world with the tooling already available.

If I'm wearing my distro author hat, I can already identify components directly by their URI. What is the registry's indirection providing me?

We talked a bit offline about the challenges of working with components that aren't available from a proper GOPROXY or aren't available publicly, but the toolchain has e.g. GOPRIVATE for that.

This will be part of the more general distrogen breakdown doc I am working on, but I'll answer briefly here.

Registry gives us the following things currently:

Allows us to use a logical name for a component rather than the whole URL in the spec.yaml

Allows redirecting a module to a local package or import path, basically propagating the options available in OCB

It will enable the following unreleased things:

Declaring a supported OTel Collector version. This looks fine right now cause we only refer to upstream, who tag the components based on the Collector release, but if anyone decides to tag things in ways that don't match upstream, this becomes confusing. We want distrogen to reconcile and complain about components it resolved that don't match the requested collector version.

Propagating necessary Go replaces. Rather than needing to go out to a component's source repo and checking if they have any replaces that they need to function properly, distrogen will be able to recognize that it's pulled a component from a registry that is requiring certain module replacements and replicate them in the OCB manifest.

Deeper explanation will be part of the design document I'm working on for the latter two.

ridwanmsharif

I'll let the two of you resolve the open threads, I think I'm okay with most of the answers given as is - we can iterate on the rest in follow up changes

# Conflicts: # cmd/distrogen/distribution.go # cmd/distrogen/testdata/generator/distribution/build_container/golden/spec.yaml

braydonk and others added 9 commits July 9, 2025 13:34

in progress

3bf532e

in progress 2

87c7461

in progress 2

caae93b

Merge remote-tracking branch 'origin/distrogen_update' into distrogen…

0d13220

…_update

debugging target

5b5e34f

Makefile bugfixes

ec10f6e

Adding tool generation

af25ab6

inprogress 3

261516e

Done with scratch

bfc50a0

avilevy18 requested a review from braydonk July 24, 2025 20:33

Merge branch 'master' into distrogen_update

95c4a03

braydonk reviewed Jul 28, 2025

View reviewed changes

avilevy18 added 2 commits July 28, 2025 20:02

Addressed comments

3c13efb

Merge remote-tracking branch 'origin/distrogen_update' into distrogen…

9fa3479

…_update

avilevy18 requested a review from braydonk July 29, 2025 19:25

braydonk reviewed Jul 30, 2025

View reviewed changes

cmd/distrogen/registry.go Outdated Show resolved Hide resolved

avilevy18 added 2 commits July 30, 2025 18:19

Merge branch 'refs/heads/master' into distrogen_update

aa77688

Adding golden tests for project level

b2d6813

avilevy18 requested a review from braydonk August 5, 2025 15:06

avilevy18 and others added 8 commits August 5, 2025 15:05

Merge branch 'master' into distrogen_update

d4993b3

Make the Makefile and VERSION file templates

c8d7f1d

Add an empty registry.yaml in project generation

47c967b

change registry generation into generation not templates

0c61169

Merge branch 'refs/heads/master' into distrogen_update

98738b9

updated build_image.gcl

df27ea8

fixed golden tests for project generation.

63c8747

Fixing goldens

903cad0

Added a registry command

218587c

avilevy18 requested a review from ridwanmsharif September 4, 2025 21:17

ridwanmsharif reviewed Sep 4, 2025

View reviewed changes

ridwanmsharif reviewed Sep 5, 2025

View reviewed changes

cmd/distrogen/testdata/generator/project/golden/spec.yaml Show resolved Hide resolved

ridwanmsharif reviewed Sep 5, 2025

View reviewed changes

avilevy18 added 3 commits September 5, 2025 15:53

component command bug fix

34dfa1d

fixed bug where it was generating both a distrogen and .distrogen fol…

d19c93a

…der.

Addressed a comment where creating the .distrogen folder manually is …

6620473

…no longer necessary

jefferbrecht reviewed Sep 8, 2025

View reviewed changes

ridwanmsharif approved these changes Sep 18, 2025

View reviewed changes

avilevy18 added 3 commits September 18, 2025 20:28

Merge branch 'refs/heads/master' into distrogen_update

f5a6888

# Conflicts: # cmd/distrogen/distribution.go # cmd/distrogen/testdata/generator/distribution/build_container/golden/spec.yaml

Rename components.go

e38aad2

Merge branch 'refs/heads/master' into distrogen_update

0e661e6

avilevy18 marked this pull request as ready for review September 23, 2025 20:04

avilevy18 added 4 commits September 23, 2025 20:56

Cleaning up scratch directories

e3f6def

Added licenses

675617d

presubmit fixes

5c847f8

presubmit fixes

ef431ed

avilevy18 merged commit c1d5f61 into master Sep 24, 2025
10 checks passed


		var ErrComponentNotFound = errors.New("component not found")

		type ComponentType string

		@@ -0,0 +1,4 @@
		module {{ .Spec.Module }}

		go {{ .Spec.RenderGoMajorVersion }}

Distrogen update #372

Distrogen update #372

Uh oh!

Conversation

avilevy18 commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

braydonk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jefferbrecht commented Aug 29, 2025

Uh oh!

ridwanmsharif left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jefferbrecht Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jefferbrecht Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ridwanmsharif left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

avilevy18 commented Jul 22, 2025 •

edited

Loading

jefferbrecht Sep 8, 2025 •

edited

Loading

jefferbrecht Sep 8, 2025 •

edited

Loading