[cmd/telemetrygen] Export telemetrygen functions for testing

[Erog38]

Description

This PR addresses #36984 in order to open up telemetrygen to be used by golang tests and generate varying amounts of telemetry in code to support use cases which don't rely on external tools.

The following major changes were made to clear the footprint for how this API can be used:

1. Moved metrics, traces, and logs under pkg instead of internal.
2. Unexported the run function for each test suite so the entrypoint is only the Start function
3. Moved config defaults from pflags into a SetDefaults function which is exported alongside NewConfig
   for changed packages in order for users to get the same sane defaults as if run from the command line.
4. Adjusted the pflags to use the existing default values rather than duplicate their own removing default config duplication
5. Added E2E tests for Metrics and Logs and adjusted Traces to use the new API so it stays consistent.
6. Adjusted log and metric exporter instantiation slightly due to a race condition with an err variable which was only caught by the new E2E tests.

Testing

Added to the E2E tests to ensure consistent API specs.

[Erog38]

This goes hand in hand with #37003 in order to help own and maintain this API. I read about chloggen, but haven't used it before so I copied the template and made a version manually. I hope I did that properly.

[codecov]

Codecov Report

Attention: Patch coverage is 15.17857% with 190 lines in your changes missing coverage. Please review.

Project coverage is 79.57%. Comparing base (1629eda) to head (a719b82).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/telemetrygen/pkg/logs/logs.go	8.88%	39 Missing and 2 partials ⚠️
cmd/telemetrygen/pkg/metrics/metrics.go	8.88%	39 Missing and 2 partials ⚠️
cmd/telemetrygen/internal/common/config.go	0.00%	29 Missing ⚠️
cmd/telemetrygen/pkg/traces/config.go	15.15%	28 Missing ⚠️
cmd/telemetrygen/pkg/logs/config.go	35.13%	24 Missing ⚠️
cmd/telemetrygen/pkg/metrics/config.go	0.00%	18 Missing ⚠️
cmd/telemetrygen/pkg/metrics/metrics_types.go	0.00%	5 Missing ⚠️
cmd/telemetrygen/pkg/traces/traces.go	33.33%	2 Missing ⚠️
cmd/telemetrygen/config.go	75.00%	1 Missing ⚠️
cmd/telemetrygen/pkg/metrics/worker.go	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #37044      +/-   ##
==========================================
- Coverage   79.60%   79.57%   -0.03%     
==========================================
  Files        2250     2250              
  Lines      211890   211951      +61     
==========================================
- Hits       168676   168668       -8     
- Misses      37540    37608      +68     
- Partials     5674     5675       +1     

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

[Erog38]

Interestingly much of the code stated as not covered in the patch is covered in the e2e tests, I suspect that they don't contribute to the coverage since it's technically a different Go module?

[Erog38]

@mx-psi, @codeboten, sorry to ping both of you, but if you have a moment in your day I'd love to get your thoughts on this implementation.

I did what I could to create a sane first API to maintain and use. Any suggestions are welcome! 😄

I did consider using Options hooks so it would look something like:

Start(
    WithEndpoint("localhost:1234"),
    WithRunDuration(5 * time.Second),
)

But I thought it wouldn't give much more flexibility than just:

mc := metrics.NewConfig()
mc.Endpoint = "localhost:1234"
mc.RunDuration = 5 * time.Second
err := metrics.Start(mc)

Which was simpler to implement and support

[Erog38]

CI runner failure, but things seem to be running alright. Interestingly I don't have permissions to re-run the failed jobs.

[Erog38]

I'd love some visibility so I can stop merging main changes 😅

[atoulme]

@mx-psi @codeboten please review as codeowners. Given that @Erog38 is also a codeowner, we can assume this is good to go.

[mx-psi]

Typically cmd/ is used for executables only. I am fine using this API, but I would like it to live outside the cmd folder. What about the top-level internal folder?

[mx-psi]

Would it be possible to make all/most of these fields private? (also applies to other signals)

[Erog38]

We could, at least unless someone comes with another use case for them to be exported.

I tended to think about exporting it all such that the config values for the telemetrygen that are exported by cmd flags are also available as API variables. This way there's parity of expected functionality if you're using the API or the cmd call.

[mx-psi]

Hm, I think keeping it private helps us design the API more carefully. I agree that functionality should be available in both ways, but how to do it may be different than this.

[Erog38]

@mx-psi that would block imports of the API, removing the benefit of exposing it. In typical go projects I agree with you, but in an effort to keep the logical components contained rather than spreading it to more of the repo, I left it in cmd a bit intentionally.

Given the spread of components contained in this monorepo it made sense to break convention. That said if it still makes more sense to move the exported logic to another location which is accessible, I can do that.

[Erog38]

I'd urge you to consider telemetrygen its own module akin to any other part of contrib exporters, receivers, etc. Where keeping it self-contained builds consistency and logical walls around the component.

[mx-psi]

@Erog38 sorry, maybe I misread the original issue. Do you want to use this for tests outside of opentelemetry-collector-contrib?

[jarias-lfx]

/easycla

[linux-foundation-easycla]

• ❌ - login: @Erog38 / name: Phil Gore . The commit (0aa0f81, 9c3c8cf, f6114b5, 1b25937, 51a2557, 9a2f91a, 8c9d69f, 20ff850, 7507493, 5112ea8, eeb2b60, 636d365, 82df599, 3ccc758, 3bdd387, 240600f, 48b5192, 9c590e0, 9ea331b, fa408b6, 21d443c, d66fdbc, c738671, ed8f7a1, 1580d8b, b2b10fa, 9139162, d5ad86b) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

[atoulme]

@Erog38 sorry, maybe I misread the original issue. Do you want to use this for tests outside of opentelemetry-collector-contrib?

see the discussion on #36984

[Erog38]

Hey @mx-psi that's right, as @atoulme mentioned, after talking in #36984 I asked about opening up this API for test use to support a pattern where integration tests can be written in pure Go to generate configurable telemetry signals pass them through a collector, and receive them into a test sink.

Using telemetrygen for this use case made sense in order to leverage the existing community effort and keep the API up to date with collector releases. It was also added to my corporate responsibilities so I have dedicated time to give a hand across all issues / PRs with telemetrygen and likely other components as it becomes possible 😄

edit: also, apologies for the delay, folks got sick here 😅

[mx-psi]

I am fine with exposing this if that's the use case, I would like @codeboten to weigh in on where this should be exposed (should it be in cmd/telemetrygen or in a pkg folder like pkg/telemetrygen?)

I will drop my 'request changes' but want to clarify this before merging. Could you look into the other comments I had?

[Erog38]

For sure! I wasn't sure if they still applied or not given the miscommunication of use case

[Erog38]

As for the CLA notice just now, I'm working through transitioning from a personal CLA to a corporate one. I have a meeting with my corp CLA manager tomorrow to get through it, but I expect no issues.