[component][componenttest] Add validation for config serialization

[Dhruvit96]

Description

This unit test enhancement ensures that the configuration can be serialized correctly.

Link to tracking issue

Fixes #12906

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: Dhruvit96 / name: Dhruvit Maniya (b12a992)

[dmathieu]

Could you add tests? (and fix the failing CI checks)

[Dhruvit96]

Could you add tests? (and fix the failing CI checks)

I don't think this change requires changelog.
For now I have added chore in PR title but I think it skipped few tests as well, could you please add skip changelog lable?

[dmathieu]

I think it does require a changelog entry. This changes the behavior of a public method in a public package.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.63%. Comparing base (dc3f5d2) to head (dca103b).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12912      +/-   ##
==========================================
+ Coverage   91.61%   91.63%   +0.01%     
==========================================
  Files         505      505              
  Lines       28409    28429      +20     
==========================================
+ Hits        26027    26051      +24     
+ Misses       1873     1870       -3     
+ Partials      509      508       -1     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Dhruvit96]

I have completed all the changes from my end, and the implementation is now ready for review. It should pass all CI checks successfully.

Thank you for your patience.

[Dhruvit96]

Please let me know if there's anything I might have missed in this PR or anything I can do to help expedite the review process.

[evan-bradley]

@Dhruvit96 Sorry for the slow review, overall this looks good to me. Could you rebase?

[Dhruvit96]

@evan-bradley I've rebased the branch, could you please have a look?

[evan-bradley]

I like the idea of checking whether a config struct can be marshalled and I think CheckConfigStruct may be a good place to add it, but I'm not so sure about adding confmap as a dependency for most components, I think all the go.mod changes in this PR shows how widely impactful this will be.

I think ultimately we should be okay since virtually all Collector binaries will need confmap anyway, but I want to make sure we're deliberate before we grow the transitive dependency list like this. @open-telemetry/collector-approvers any thoughts?

[evan-bradley]

From the 2025-05-21 SIG meeting: We think that since YAML is a superset of JSON, and confmap maintains close compatibility with YAML, we can get by with just testing JSON now to reduce the number of added dependencies.

@Dhruvit96 Could you revise this PR so it only checks the output of json.Marshal? We think this will likely be sufficient for now and we can address any discrepancies between JSON and confmap or YAML in a separate PR.

[Dhruvit96]

Hi @evan-bradley,

I've updated the PR to validate only JSON marshalling. However, this change did not reduce dependencies in any package other than the scrapertest.

I agree with your point that YAML is a superset of JSON, and we might be able to rely on JSON validation alone. That said, since the dependency impact is similar, validating both JSON and YAML would make the testing more comprehensive.

Could you please share your thoughts?

[evan-bradley]

Suggested change

	conf := confmap.New()
	if err := conf.Marshal(config); err != nil {
	return err
	}
	configMap := conf.ToStringMap()
	_, err := json.Marshal(configMap)
	_, err := json.Marshal(config)

If we remove marshaling with confmap, we should be able to only test JSON marshaling here.

[Dhruvit96]

This PR introduces a generic safeguard to prevent panics during the marshalling of configuration maps. We observed that the OpenTelemetry Collector encountered panics when used in conjunction with the OpAMP extension. As identified in this comment, the root cause was a failure to marshal a configuration represented as a map[string]any.

Validating only the marshallability of structs does not address such scenarios, as marshalling errors in structs can often be resolved using the json:"-" tag. However, in the case of maps, resolving the issue requires the use of the mapstructure:"-" tag.

[Dhruvit96]

@evan-bradley Based on the information provided above, could you please let me know the next steps?

[evan-bradley]

Thanks @Dhruvit96. The dependency impact is coming from the use of confmap, we can remove that as well. I left a suggestion that shows how I think it should look.

[github-actions]

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

[github-actions]

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

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.