Use test host in unit tests

[ngruson]

Fixes issue #4976

Changes

@JamesNK gave some recommendations about the usage of WebApplicationFactory.
As much as possible, we have implemented that.

Some test cases require a KestrelServer implementation for Kestrel metrics to be emitted.
In those cases, a WebApplication instance is instantiated to run the unit test against.
Previously, the hardcoded port number 5000 was used to run the host.
The port number is now made variable to isolate the unit test as much as possible.

Merge requirement checklist

• CONTRIBUTING guidelines followed (nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: ngruson / name: Nils Gruson (df119ed, 41e4055, 56099ba, 5386959, 77e9684, 66d97cb, 3e6af87, 1ecc29c, e153d88, dd5c16a, 6561139, ec2bb92, d508cac, 02bcf0b, 56aa1db, 1b69355, a60a8f5, 54340de, 79660f6, 895da60, 12aeef1, cdecae3, 3e7c3bf, 6024f19, dd15e63, fabc1c6, cbb5320, cf9bc01, 44639b4, 8952aca, f8c43fb, 6227ab0, bac5a61, 6524198, 10c93cc, ebe17a2, 2ff66d5, bbe10ec, 292da0f)
• ✅ login: utpilla / name: Utkarsh Umesan Pillai (4f161df)
• ✅ login: vishweshbankwar / name: Vishwesh Bankwar (7add987)

[vishweshbankwar]

@ngruson - Thank you for taking this. I have not used MetricCollector before. But looking at the link shared in #4976, I think it may not be something we can use for testing our scenario.

MetricCollector is testing metrics outside of OTel context by enabling its own MeterListener. In our case we want to validate if the user configures OTel SDK and instrumentation as shown below, will they get the desired metrics on .net8.0 exported?

Sdk.CreateMeterProviderBuilder()
   .AddAspNetCoreInstrumentation() 
   .Build();

In your test assert on collector is independent of assert on exported Items

cc: @JamesNK

[vishweshbankwar]

regarding : https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/InProcServerTests.cs: We should delete this file as the test was added just for e.g. purpose. You could send a separate PR to remove that one.

[ngruson]

Other files that could be refactored to use the test host:

• AspNetCoreInstrumentationBenchmarks.cs
• AspNetCoreInstrumentationNewBenchmarks
• HttpClientInstrumentationBenchmarks
• BasicTests.cs
• W3CTraceContextTests.cs (don't know if this can be done in similar fashion. The endpoint is passed to a Python script.)

[vishweshbankwar]

Other files that could be refactored to use the test host:

• AspNetCoreInstrumentationBenchmarks.cs
• AspNetCoreInstrumentationNewBenchmarks
• HttpClientInstrumentationBenchmarks
• BasicTests.cs
• W3CTraceContextTests.cs (don't know if this can be done in similar fashion. The endpoint is passed to a Python script.)

@ngruson Yes, it would be great if you could do that refactor. We don't need to refactor the Benchmarks one for now. BasicTests already uses WebApplicationFactory in most of the tests, we need to refactor the ones not using it like this one

W3CTraceContextTests.cs (don't know if this can be done in similar fashion. The endpoint is passed to a Python script.)

It should be possible. May be try that in separate PR than BasicTests.cs one?

[ngruson]

@vishweshbankwar :
I resolved some merge conflicts and now the builds are passing.
Can you see if this is ok?

[vishweshbankwar]

LGTM: Thanks @ngruson! Please clean up the PR description as well and describe the actual changes.

[ngruson]

LGTM: Thanks @ngruson! Please clean up the PR description as well and describe the actual changes.

Just updated the PR description, is it sufficient?
You probably don't update the change log for this?

[vishweshbankwar]

LGTM: Thanks @ngruson! Please clean up the PR description as well and describe the actual changes.

Just updated the PR description, is it sufficient? You probably don't update the change log for this?

Correct, changelog updates are not done for unit test changes.