[service] Fix panic when not using NewCommand

[mx-psi]

Description:

• Reorder tests to trigger bug
• Add default value to flags

Link to tracking Issue: Fixes #4138

Testing: Reordered tests to trigger the panic

[bogdandrutu]

@mx-psi it is still expected to crash since the issue #4138 is not yet resolved, but I believe this is good to speed-up that issue.

[codecov]

Codecov Report

Merging #4139 (db35aea) into main (afad36e) will not change coverage.
The diff coverage is 70.00%.

@@           Coverage Diff           @@
##             main    #4139   +/-   ##
=======================================
  Coverage   87.66%   87.66%           
=======================================
  Files         174      174           
  Lines       10224    10224           
=======================================
  Hits         8963     8963           
  Misses       1011     1011           
  Partials      250      250           

Impacted Files	Coverage Δ
service/internal/telemetrylogs/logger.go	71.42% <50.00%> (ø)
internal/collector/telemetry/telemetry.go	100.00% <100.00%> (ø)
service/flags.go	72.00% <100.00%> (ø)

---

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 afad36e...db35aea. Read the comment docs.

[tigrannajaryan]

Does this fully fix #4138 or there is more work to do? If fixed would be nice to add a test to show it is fixed.

[mx-psi]

Does this fully fix #4138 or there is more work to do?

Yes, the example program does not crash at startup and in general this class of crashes should be gone.

If fixed would be nice to add a test to show it is fixed.

I thought I needed to do more work to work around collectorTelemetry for adding a test, but it was easier than I expected, added in the last commit :)

[Aneurysm9]

Suggested change

	metricsAddrPtr = &metricsAddrDefault
	metricsPrefixPtr = &metricsPrefixDefault
	metricsAddrPtr = new(string)
	metricsPrefixPtr = new(string)

Would this be safer? Does anything use the default value that might break if it changes when the user specifies this value via CLI flags?

[mx-psi]

On this particular instance it would be fine to just use new, but on the logs side it's not

opentelemetry-collector/service/internal/telemetrylogs/logger.go

Lines 78 to 90 in afad36e

	func logDeprecatedMessages(logger *zap.Logger) {
	if *loggerLevelPtr != "deprecated" {
	logger.Warn("`log-level` command line option has been deprecated. Use `service::telemetry::logs` in config instead!")
	}
	if *loggerProfilePtr != "deprecated" {
	logger.Warn("`log-profile` command line option has been deprecated. Use `service::telemetry::logs` in config instead!")
	}
	if *loggerFormatPtr != "deprecated" {
	logger.Warn("`log-format` command line option has been deprecated. Use `service::telemetry::logs` in config instead!")
	}
	}

since you need the value to be "deprecated". I would prefer to leave it like this for consistency