Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename otel.trace.classes.excludes to otel.javaagent.exclude-classes #1747

Merged
merged 2 commits into from
Nov 25, 2020

Conversation

trask
Copy link
Member

@trask trask commented Nov 24, 2020

Missed this in #1740.

See latest note about exclude-classes vs excludeClasses: #1414 (comment), though happy to change to something else either in this PR or later on

@iNikem
Copy link
Contributor

iNikem commented Nov 24, 2020

I think we should fix our property->env mapping to support excludeAll. This exclude-all is quite unnatural for Java I would say.

@mateuszrzeszutek
Copy link
Member

Hmm, we use dash-separated words in the instrumentation names though: e.g. spring-webmvc.
Still, I agree that excludeAll looks more Java-like in this case.

@trask
Copy link
Member Author

trask commented Nov 24, 2020

I have the same impression about camelCase being more Java-like, but I went looking at the Spring Boot env var / system property mappings, and they support both camelCase and kebab-case, but recommend kebab-case:

Property Note
acme.my-project.person.first-name Kebab case, which is recommended for use in .properties and .yml files.

And also interestingly:

The prefix value for the annotation must be in kebab case (lowercase and separated by -, such as acme.my-project.person).

So now I'm thinking kebab case is the way to go (and as benefit is consistent with our module names when used in the property names, e.g. otel.instrumentation.external-annotations.exclude-methods).

@iNikem iNikem merged commit 325238f into open-telemetry:master Nov 25, 2020
@trask trask deleted the rename-another-otel-property branch November 26, 2020 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants