[confignet] Mark as stable

[TylerHelmuth]

Description:
Mark confignet as Stable

Link to tracking Issue:
Closes #9045

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.23%. Comparing base (95f9cec) to head (a8d60b8).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #9801   +/-   ##
=======================================
  Coverage   92.23%   92.23%           
=======================================
  Files         414      414           
  Lines       19810    19810           
=======================================
  Hits        18271    18271           
  Misses       1167     1167           
  Partials      372      372           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[atoulme]

LGTM! I have reviewed the code and the test coverage, everything looks great.

[dmitryax]

I've been thinking how to reduce code duplication between TCPAddrConfig and AddrConfig, but cannot see a clean simple way. Go isn't very flexible sometimes.

[mx-psi]

By stabilizing this, we commit to two things from confmap:

1. Supporting mapstructure YAML tags. I am fine with that, I don't think there is much of a point on changing it, but I know there has been some discussion about this (@atoulme may have more details?). Is this resolved?
2. Supporting UnmarshalText on config unmarshaling. This is a standard library interface, so I think it is a pretty safe assumption.

We also commit to one thing from component:

1. AddrConfig implements component.ConfigValidator:

   opentelemetry-collector/config/confignet/confignet.go

   Line 106 in 15201f1

   func (na *AddrConfig) Validate() error {

   . We are commiting to keep on supporting that interface.

For the implicit dependency on component, I don't really get why we are adding Validate(): the UnmarshalText method from the transport should already do validation at unmarshaling time. Shouldn't that be enough? If we can remove that method, we eliminate one implicit dependency.

[TylerHelmuth]

I don't really get why we are adding Validate()

@mx-psi I believe Validate() should be included since AddrConfig.Transport is a public field. Users are able to set AddrConfig.Transport manually, bypassing TransportType.UnmarshalText. See #9385 (comment).

[TylerHelmuth]

@mx-psi based on your comments it sounds like we have sufficient dependencies on component and confmap to hold off marking this stable until those 2 modules are stable.

[mx-psi]

@mx-psi based on your comments it sounds like we have sufficient dependencies on component and confmap to hold off marking this stable until those 2 modules are stable.

Or at least I think if we merge this, we should have a way to validate that we are not changing these assumptions

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[TylerHelmuth]

Will reopen once component and confmap are stable

[atoulme]

There is no dependency to component left and confmap is stable, reopening.

[mx-psi]

I think it's pretty clear that Validate is here to stay, which was the only dependency left with component. I vote we merge this.

@open-telemetry/collector-approvers I will merge this after v0.109.0 is released unless somebody blocks the PR