Remove component provider generic type

[trask]

The generic type makes it awkward to use with @AutoService, because it requires you to use SuppressWarnings("rawtypes") (some background at google/auto#870), e.g.

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/c7dd76454f5af080a1c385fdb7fda2c6bace68f5/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/internal/ProcessResourceComponentProvider.java#L19-L20

Plus, it doesn't seem like we lose much by removing the generic type (in fact it ends up being slightly less code).

[codecov]

Codecov Report

❌ Patch coverage is 82.75862% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.16%. Comparing base (36ca9b8) to head (0a14e97).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
...incubator/fileconfig/DeclarativeConfigContext.java	44.44%	4 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7606      +/-   ##
============================================
- Coverage     90.18%   90.16%   -0.03%     
+ Complexity     7226     7225       -1     
============================================
  Files           820      820              
  Lines         21790    21794       +4     
  Branches       2135     2136       +1     
============================================
- Hits          19651    19650       -1     
- Misses         1468     1472       +4     
- Partials        671      672       +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.

[trask]

this is just removing an existing duplicative line

[trask]

this is just removing an existing duplicative line

[breedx-splk]

Thanks, this looks good to me.

Java generics strike again..... 🙈

[breedx-splk]

bummer, but yeah ok.

[jkwatson]

I'm fine with it. Don't know if we want to wait for @jack-berg or ask for forgiveness if he hates the change?

[zeitlinger]

that feels very odd to me - like deliberately defeating the type checker

[zeitlinger]

I don't think this is a good idea - in makes the API less clear than it is now

[robsunday]

It is great you attempted to solve this coding issue, however I'd say that old implementation has some important strengths that are missing in new generic-less approach.
I mean - with a new approach there is nothing preventing developers from introducing getType() and create() return type inconsistencies.
With new approach it is possible to declare component provider like this:

public final class ConsoleMetricExporterComponentProvider
    implements ComponentProvider {

  @Override
  public Class<?> getType() {
    return SpanProcessor.class;
  }

  @Override
  public String getName() {
    return "console";
  }

  @Override
  public Object create(DeclarativeConfigProperties config) {
    return LoggingMetricExporter.create();
  }
}

Maybe it's just me, but in my opinion this API becomes somehow "fragile" now.
I know that exception will be thrown at runtime, however I think it is better to add one more annotation to silence some warnings in concrete ComponentProvider implementations.

[brunobat]

This will cause disruption on PPL implementing their own exporters.
Do we really need to do this?

[laurit]

I'd support this. In instrumentation repo after changing a module that contains a ComponentProvider build often fails with warning: Service provider io.opentelemetry.sdk.autoconfigure.spi.internal.ComponentProvider is generic, so it can't be named exactly by @AutoService. If this is OK, add @SuppressWarnings("rawtypes"). Which is quite annoying, cleaning the affected module gets the build going again.

[trask]

@jack-berg @jkwatson reopening because this is two problems now that this generic type is causing for downstream consumers

[trask]

I'd support this. In instrumentation repo after changing a module that contains a ComponentProvider build often fails with warning: Service provider io.opentelemetry.sdk.autoconfigure.spi.internal.ComponentProvider is generic, so it can't be named exactly by @AutoService. If this is OK, add @SuppressWarnings("rawtypes"). Which is quite annoying, cleaning the affected module gets the build going again.

ugh, this just bit me today

[jack-berg]

I don't hate this. Its a bit more error prone than the generic version, but a bit more user friendly because it avoids errors with autoservice tooling. Its weird that autoservice throws this error given how popular generic types are for other common autoservice use cases like DAOs. 🤷

**Sorry wrote this review on 10/28 but apparently forgot to hit the submit button 🤦 **

[jack-berg]

I think we need to add an additional check here to check if create returns the same type as getType().

[trask]

0a14e97

[zeitlinger]

great - that restores the type safety (even though only at runtime)