-
Notifications
You must be signed in to change notification settings - Fork 199
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
Add span kind to sampling overrides #1960
Merged
Merged
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
cf5b048
Option 1
trask aa88f25
Revert "Option 1"
trask 02d04ad
Add span kind to sampling overrides
trask 4b99b4c
Support key-only matches
trask 48bfc9f
Fix
trask 1332abd
Bump version
trask dcd7721
Fix sporadic test failure
trask 8259d1d
Fix test
trask 8553cae
Add todo
trask File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,7 @@ | |
import java.util.Map; | ||
import java.util.regex.Pattern; | ||
import java.util.regex.PatternSyntaxException; | ||
import org.checkerframework.checker.nullness.qual.Nullable; | ||
|
||
// an assumption is made throughout this file that user will not explicitly use `null` value in json | ||
// file | ||
|
@@ -62,6 +63,25 @@ private static boolean isEmpty(String str) { | |
return str == null || str.trim().isEmpty(); | ||
} | ||
|
||
public enum SpanKind { | ||
@JsonProperty("server") | ||
SERVER(io.opentelemetry.api.trace.SpanKind.SERVER), | ||
@JsonProperty("client") | ||
CLIENT(io.opentelemetry.api.trace.SpanKind.CLIENT), | ||
@JsonProperty("consumer") | ||
CONSUMER(io.opentelemetry.api.trace.SpanKind.CONSUMER), | ||
@JsonProperty("producer") | ||
PRODUCER(io.opentelemetry.api.trace.SpanKind.PRODUCER), | ||
@JsonProperty("internal") | ||
INTERNAL(io.opentelemetry.api.trace.SpanKind.INTERNAL); | ||
|
||
public final io.opentelemetry.api.trace.SpanKind otelSpanKind; | ||
|
||
SpanKind(io.opentelemetry.api.trace.SpanKind otelSpanKind) { | ||
this.otelSpanKind = otelSpanKind; | ||
} | ||
} | ||
|
||
public enum MatchType { | ||
@JsonProperty("strict") | ||
STRICT, | ||
|
@@ -373,6 +393,8 @@ private static String getDefaultPath() { | |
} | ||
|
||
public static class SamplingOverride { | ||
// TODO (trask) consider making this required when moving out of preview | ||
@Nullable public SpanKind spanKind; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. declare it as io.opentelemetry.api.trace.SpanKind? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see comment above |
||
// not using include/exclude, because you can still get exclude with this by adding a second | ||
// (exclude) override above it | ||
// (since only the first matching override is used) | ||
|
@@ -381,11 +403,11 @@ public static class SamplingOverride { | |
public String id; // optional, used for debugging purposes only | ||
|
||
public void validate() { | ||
if (attributes.isEmpty()) { | ||
if (spanKind == null && attributes.isEmpty()) { | ||
// TODO add doc and go link, similar to telemetry processors | ||
throw new FriendlyException( | ||
"A sampling override configuration has no attributes.", | ||
"Please provide one or more attributes for the sampling override configuration."); | ||
"A sampling override configuration is missing \"spanKind\" and has no attributes.", | ||
"Please provide at least one of \"spanKind\" or \"attributes\" for the sampling override configuration."); | ||
} | ||
if (percentage == null) { | ||
// TODO add doc and go link, similar to telemetry processors | ||
|
@@ -407,8 +429,8 @@ public void validate() { | |
|
||
public static class SamplingOverrideAttribute { | ||
public String key; | ||
public String value; | ||
public MatchType matchType; | ||
@Nullable public String value; | ||
@Nullable public MatchType matchType; | ||
|
||
private void validate() { | ||
if (isEmpty(key)) { | ||
|
@@ -417,9 +439,9 @@ private void validate() { | |
"A sampling override configuration has an attribute section that is missing a \"key\".", | ||
"Please provide a \"key\" under the attribute section of the sampling override configuration."); | ||
} | ||
if (matchType == null) { | ||
if (matchType == null && value != null) { | ||
throw new FriendlyException( | ||
"A sampling override configuration has an attribute section that is missing a \"matchType\".", | ||
"A sampling override configuration has an attribute section with a \"value\" that is missing a \"matchType\".", | ||
"Please provide a \"matchType\" under the attribute section of the sampling override configuration."); | ||
} | ||
if (matchType == MatchType.REGEXP) { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is io.opentelemetry.api.trace.SpanKind an enum? can we reuse it rather than create a new one? if their spankind is updated, we shouldn't need to update ours.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
our convention is for json enum values to be lowercase, and we can't add
@JsonProperty
to those classes since they are not under our control. I will add a TODO marker here to investigate other options for mapping external annotations to lowercase.