Skip to content

Move config validate#897

Merged
christos68k merged 9 commits into
open-telemetry:mainfrom
ogaca-dd:move-config-validate
Nov 7, 2025
Merged

Move config validate#897
christos68k merged 9 commits into
open-telemetry:mainfrom
ogaca-dd:move-config-validate

Conversation

@ogaca-dd
Copy link
Copy Markdown
Contributor

@ogaca-dd ogaca-dd commented Oct 28, 2025

Move controller.Config.Validate content to config.Config.Validate.

This is a split of #859 where the goal is to implement Validate on the config without code duplication. See #849 (comment) for more context.

Comment thread collector/config/config.go Outdated
@ogaca-dd ogaca-dd marked this pull request as ready for review October 30, 2025 14:47
@ogaca-dd ogaca-dd requested review from a team as code owners October 30, 2025 14:47
@ogaca-dd ogaca-dd requested a review from florianl November 3, 2025 15:55
Comment thread collector/config/config.go Outdated
ogaca-dd and others added 2 commits November 4, 2025 11:23
Co-authored-by: Florian Lehner <florianl@users.noreply.github.com>
Copy link
Copy Markdown
Member

@florianl florianl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update. Can you resolve also the merge conflict?

Comment thread collector/config/config.go Outdated
"runtime"
"time"

"go.opentelemetry.io/collector/confmap/xconfmap"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like with the import of this dependency we increase the binary size from 30108535 bytes (for commit e413d8c on main) to 30929253 bytes (for commit f6f355b on this branch). Resulting in an increase of ~0.8 MB.

As xconfmap is used only for the compile time check, can we work around this in some other way?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed this type assertion as this is also tested by this unit test.

@ogaca-dd ogaca-dd requested a review from florianl November 6, 2025 12:31
@christos68k christos68k merged commit a47b46a into open-telemetry:main Nov 7, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants