Add minimum_severity and trace_based logger configuration parameters

[trask]

Alternative to #4611

Some related prior discussions:

• Stabilize Logger.Enabled #4208 (comment)
• Specify how Logs SDK implements Enabled #4207 (comment)

Java POC @ open-telemetry/opentelemetry-java#7529

Declarative config for this is a bit nicer than in #4611, also supports applying different minimum severity levels to different loggers:

file_format: "1.0"

logger_provider:
  logger_configurator/development:
    default_config:
      minimum_severity: WARN
      trace_based: true
    loggers:
      - name: "com.example.app.*"
        config:
          minimum_severity: INFO
      - name: "com.example.db.*"
        config:
          minimum_severity: ERROR

[pellared]

This does not support having different minimum severity levels for different processors (processing pipelines).
Isn't this can be critical/very important for OTel Collector?

From open-telemetry/opentelemetry-go-contrib#7549:

opentelemetry-collector enables users to configure its internal telemetry using otelconf. [...] For example, we need to configure leveled logging, which could theoretically be done with the minsev processor

CC @codeboten

It is possible to achieve the functionality of "logger configurator" using processors (e.g. logger filter). The question is whether we need a configuration option that is redundant? Moreover, using processors it is possible to compose more complex pipelines that are not possible using the logger configurator.

It should be possible to "implement" the declarative config of logger configurator using processors. Then the logger configurator would be a facade for configuring some processors.

PS.I think there were some similar/related discussions here #4439

If we are looking into simplicity then another alternative (mainly for default_config) could be setting the minimum severity level on the whole logger provider. This would require even "less config" but it would be not flexible.

[pellared]

Seems like there is a consensus to go forward with this proposal.

It looks like Collector already handles filtering using filterprocessor.

[pellared]

We discussed this during Go SIG meeting.
We are fine with the proposal given we want to implement this configuration using (internally) LogRecordProcessors. We do not want to add LoggerConfigurator into the Go Logs SDK, because we find it unnecessary as everything should be achievable using LogRecordProcessor. The experience for the end-user should remain the same.

CC @open-telemetry/go-triagers

[github-actions]

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

[pellared]

Suggested change

	affected by this parameter by default.
	affected by this parameter.

[trask]

see @lmolkova's concern here: #4612 (comment)

[lmolkova]

I assume @pellared's point is that SHOULD NOT is permissive enough so that by default is redundant. I still feel that 'by default' makes spec easier to read and explains intent for it to be configurable down the road, but if anyone feels strong about removing it, go ahead, I don't mind.

[trask]

@lmolkova check out #4612 (comment)

[lmolkova]

good catch!

@pellared

Log records with an unspecified severity or severity set to 0 MUST NOT be affected by this parameter.

sounds too restrictive - like we would never drop records with 0 severity (and would never have config option for it) - I don't want to close this door.

[pellared]

@lmolkova

sounds too restrictive - like we would never drop records with 0 severity (and would never have config option for it) - I don't want to close this door.

I do not agree. This is only describing the behavior of minimum_severity parameter.

MUST NOT be affected by this parameter.

We can introduce another parameter in future e.g. drop_unset_severity if users want to drop log records with unspecified severity level. The behavior of minimum_severity should be consistent in all SDKs.

[carlosalberto]

Is this expected to be stable from the start?

[carlosalberto]

Maybe a more explicit name? is trace_sample_based too mouthful? Not a strong feeling but trace_based may not be clear/obvious for new users.

[trask]

there was some prior discussion around this name: #4290 (comment)

though certainly we can change it if others feel similarly

[trask]

Is this expected to be stable from the start?

no, I checked and I think most of it is under Development sections already, did find one place where it was missed: c68bba4