Remove reference to defaultcomponents in core and deprecate include_core flag

[JamesJHPark]

Description:
This PR is to remove reference to defaultcomponents in core and deprecate the include_core flag in builder.

Link to tracking Issue:
Issue #3927

[dashpole]

This removes a lot of unit tests. Is there a reason they are no longer needed, or should we add them back in in the new location?

[bogdandrutu]

@dashpole they are already copied to contrib https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/internal/components/extensions_test.go

[dashpole]

Got it, thanks.

[codecov]

Codecov Report

Merging #4087 (5f9b4e0) into main (42566a6) will decrease coverage by 0.17%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #4087      +/-   ##
==========================================
- Coverage   90.67%   90.50%   -0.18%     
==========================================
  Files         179      178       -1     
  Lines       10414    10455      +41     
==========================================
+ Hits         9443     9462      +19     
- Misses        755      776      +21     
- Partials      216      217       +1     

Impacted Files	Coverage Δ
cmd/builder/internal/builder/config.go	85.52% <100.00%> (+1.01%)	⬆️
internal/testcomponents/example_factories.go	79.54% <100.00%> (+48.77%)	⬆️
exporter/loggingexporter/known_sync_error.go	0.00% <0.00%> (-80.00%)	⬇️
exporter/loggingexporter/logging_exporter.go	88.04% <0.00%> (-5.44%)	⬇️
receiver/otlpreceiver/otlp.go	76.00% <0.00%> (-3.58%)	⬇️
processor/memorylimiterprocessor/memorylimiter.go	85.32% <0.00%> (-3.27%)	⬇️
config/confighttp/confighttp.go	100.00% <0.00%> (ø)
service/defaultcomponents/defaults.go
exporter/otlphttpexporter/otlp.go	82.92% <0.00%> (+0.87%)	⬆️
exporter/otlpexporter/otlp.go	74.41% <0.00%> (+1.57%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 42566a6...5f9b4e0. Read the comment docs.

[github-actions]

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

[JamesJHPark]

Hi @bogdandrutu, with reference to the discussions, I have made the following changes:


1. Partially removed dependency on the real components and removed reference to defaultcomponents in tests (exception: cmd/otelcol/main.go file). For discussion, the function call to assertZPages in the test requires zpages extension. To address this, I have extracted the components of testdata/otelcol-config.yaml to a function in testcomponents package. I have also replaced the reference to defaultcomponents with this function in collector_windows_test.go file.
2. Since the collector-builder and collector-contrib depend on defaultcomponents from the core repository, I have not removed the defaultcomponents package. I have removed the tests in this package as they have already been copied to contrib. 


Please let me know if there may be a recommended approach/fixes to address the issue for next steps. Thank you.

[alolita]

@Aneurysm9 @codeboten can you pl re-review?

[jpkrohling]

As discussed during the SIG meeting today, we need first to get the builder to deprecate/remove the include_core flag, as new distributions will then not have core components in by default. When that flag is set to true, we use the default components, hence the dependency. Contrib might also use the default components, so, that needs changing as well. Once we have a release of the builder out with the new default, we can then merge this.

[jpkrohling]

You might need to set the default value of the flag in DefaultConfig. This would solve your nil pointer error. Also, make sure the distribution is indeed not having the default components, as I believe there might be a change needed in the templates.

[JamesJHPark]

You might need to set the default value of the flag in DefaultConfig. This would solve your nil pointer error. Also, make sure the distribution is indeed not having the default components, as I believe there might be a change needed in the templates.

Thank you for your reply. I have set the default value of the flag in the DefaultConfig to fix the nil pointer dereference issue but the nocore test failed with duplicate components. 
This may be related to dereferencing .Distribution.IncludeCore to a boolean value and to use this value in the pipeline in the go.tmpl file.

I have implemented a suggested approach to include the warn log before unmarshalling the config and the MarkDeprecated flag message with IncludeCore set to bool. Please let me know if these changes may be suitable to achieve the same goals to deprecate the include_core flag.

[jpkrohling]

I think those are my final concerns, after this, this will be ready to be merged.

[sodabrew]

I no longer understand what to put in my builder.yml to get the core modules back, or which ones were default-in that I was relying upon without knowing it. Would appreciate more clarity in the builder's README file on this.

Edit: I copy pastad from this example and now it's working
https://github.com/open-telemetry/opentelemetry-collector/blob/main/cmd/otelcorecol/builder-config.yaml

[jpkrohling]

I apologize, this change should have been better communicated. This PR brought clarity to the readme: #4664

If you think there's still something we should document elsewhere, let me know.

[sodabrew]

Thank you for the README example improvement!