feat: Replace obfuscate setting with an exclude attribute setting

[thompson-tomo]

This introduces an exclude_dbquerytext setting which on stable/dup excludes the db.query.text attribute to the span, otherwise it is included.

This change is to bring closer alignment to how declarative config defines settings which is attributes are included/excluded. This change also ensures un-obfusicated queries can not be captured as per semconv.

It is important to follow semconv as close as possible for consistency and in the future leverage automation

Coverage report before change:

Line Coverage: 98.31% (233 / 237)

Coverage report after change:

Line Coverage: 98.28% (229 / 233)

Which is a marginal drop caused by less lines but no increase in uncovered lines.

[hannahramadan]

hi @thompson-tomo!

Declarative Config - Do you have a link to the schema field this is mirroring?

Obfuscating Queries - Looking at the the spec

Non-parameterized query text SHOULD NOT be collected by default unless there is sanitization that excludes sensitive data, e.g. by redacting all literal values present in the query text.

This seems to say we shouldn't be capturing by default, not that we should remove the option. Queries are already obfuscated by default, so it seems the existing behavior satisfies the requirement.

[thompson-tomo]

So this is mirroring https://github.com/open-telemetry/opentelemetry-configuration/blob/main/schema-docs.md#includeexclude which is already explicitly documented for metrics, resources etc.

Non-parameterized query text SHOULD NOT be collected by default unless there is sanitization that excludes sensitive data, e.g. by redacting all literal values present in the query text.

This seems to say we shouldn't be capturing by default, not that we should remove the option. Queries are already obfuscated by default, so it seems the existing behavior satisfies the requirement.

For me this is saying we should only capture by default if we have a parameterized query or sanitization is applied. We satisfy this.

The exclude setting preserves the drop option of the previous obfuscate setting and recommended attributes by design can be excluded/removed.

The difference to the current solution is:

• removal of the ability to capture unsanitized queries.
• replaced enum option with boolean

[hannahramadan]

@thompson-tomo Thanks for that link! Looking at the schema, included/excluded is an array of attribute name patterns. So I imagine it would look something like excluded: ['db.query.text'] in the config. A single boolean named exclude_dbquerytext is a different structure, so I don't see how this is mirroring the schema, but I could be missing something.

Re: removal of the ability to capture unsanitized queries. I'm reading the spec as governing the default behavior, not requiring removal of the option to include un-obfuscated queries. Is there other spec language you're reading as prohibiting the option entirely?

[thompson-tomo]

So I imagine it would look something like excluded: ['db.query.text'] in the config. A single boolean named exclude_dbquerytext is a different structure, so I don't see how this is mirroring the schema, but I could be missing something.

It is a slightly different structure ie name is appended to key and value becomes true. However the key thing is the behaviour is the same ie you configure using attribute name what is excluded.

I am more than happy to switch to the array approach now and lose the strongly typing or internally transform it from strongly typed to loose.

In relation to unsanitized the intention of the spec is if you want a value you can should be listing which explicit parameters you want and they will be captured. I see that as a seperate PR as we are now needing to change the processor to emit an array of parameters which have had their value sanitised.