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

Allow views to select on instrument unit #5255

Merged
merged 1 commit into from
Mar 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import com.google.auto.value.AutoValue
import japicmp.model.JApiChangeStatus
import japicmp.model.JApiClass
import japicmp.model.JApiCompatibility
import japicmp.model.JApiCompatibilityChange
import japicmp.model.JApiMethod
Expand Down Expand Up @@ -34,12 +35,18 @@ val latestReleasedVersion: String by lazy {

class AllowNewAbstractMethodOnAutovalueClasses : AbstractRecordingSeenMembers() {
override fun maybeAddViolation(member: JApiCompatibility): Violation? {
if (member.compatibilityChanges == listOf(JApiCompatibilityChange.METHOD_ABSTRACT_ADDED_TO_CLASS) &&
val allowableAutovalueChanges = setOf(JApiCompatibilityChange.METHOD_ABSTRACT_ADDED_TO_CLASS, JApiCompatibilityChange.METHOD_ADDED_TO_PUBLIC_CLASS)
if (member.compatibilityChanges.filter { !allowableAutovalueChanges.contains(it) }.isEmpty() &&
member is JApiMethod &&
member.getjApiClass().newClass.get().getAnnotation(AutoValue::class.java) != null
) {
return Violation.accept(member, "Autovalue will automatically add implementation")
}
if (member.compatibilityChanges.isEmpty() &&
member is JApiClass &&
member.newClass.get().getAnnotation(AutoValue::class.java) != null) {
return Violation.accept(member, "Autovalue class modification is allowed")
}
return null
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,8 @@
Comparing source compatibility of against
No changes.
**** MODIFIED CLASS: PUBLIC ABSTRACT io.opentelemetry.sdk.metrics.InstrumentSelector (not serializable)
=== CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+++* NEW METHOD: PUBLIC(+) ABSTRACT(+) java.lang.String getInstrumentUnit()
+++ NEW ANNOTATION: javax.annotation.Nullable
*** MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.sdk.metrics.InstrumentSelectorBuilder (not serializable)
=== CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+++ NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.metrics.InstrumentSelectorBuilder setUnit(java.lang.String)
2 changes: 2 additions & 0 deletions sdk-extensions/incubator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ For example, suppose `/Users/user123/view.yaml` has the following content:
- selector:
instrument_name: my-instrument
instrument_type: COUNTER
instrument_unit: ms
meter_name: my-meter
meter_version: 1.0.0
meter_schema_url: http://example.com
Expand All @@ -36,6 +37,7 @@ SdkMeterProvider.builder()
InstrumentSelector.builder()
.setName("my-instrument")
.setType(InstrumentType.COUNTER)
.setUnit("ms")
.setMeterName("my-meter")
.setMeterVersion("1.0.0")
.setMeterSchemaUrl("http://example.com")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ static AutoValue_SelectorSpecification.Builder builder() {
@Nullable
abstract InstrumentType getInstrumentType();

@Nullable
abstract String getInstrumentUnit();

@Nullable
abstract String getMeterName();

Expand All @@ -37,6 +40,8 @@ interface Builder {

Builder instrumentType(@Nullable InstrumentType instrumentType);

Builder instrumentUnit(@Nullable String instrumentUnit);

Builder meterName(@Nullable String meterName);

Builder meterVersion(@Nullable String meterVersion);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
* - selector:
* instrument_name: my-instrument
* instrument_type: COUNTER
* instrument_unit: ms
* meter_name: my-meter
* meter_version: 1.0.0
* meter_schema_url: http://example.com
Expand All @@ -58,6 +59,7 @@
* InstrumentSelector.builder()
* .setName("my-instrument")
* .setType(InstrumentType.COUNTER)
* .setUnit("ms")
* .setMeterName("my-meter")
* .setMeterVersion("1.0.0")
* .setMeterSchemaUrl("http://example.com")
Expand Down Expand Up @@ -125,6 +127,7 @@ static List<ViewConfigSpecification> loadViewConfig(InputStream inputStream) {
SelectorSpecification.builder()
.instrumentName(getAsType(selectorSpecMap, "instrument_name", String.class))
.instrumentType(instrumentType)
.instrumentUnit(getAsType(selectorSpecMap, "instrument_unit", String.class))
.meterName(getAsType(selectorSpecMap, "meter_name", String.class))
.meterVersion(getAsType(selectorSpecMap, "meter_version", String.class))
.meterSchemaUrl(
Expand Down Expand Up @@ -248,6 +251,10 @@ static InstrumentSelector toInstrumentSelector(SelectorSpecification selectorSpe
if (instrumentType != null) {
builder.setType(instrumentType);
}
String instrumentUnit = selectorSpec.getInstrumentUnit();
if (instrumentUnit != null) {
builder.setUnit(instrumentUnit);
}

String meterName = selectorSpec.getMeterName();
if (meterName != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,14 @@ void customizeMeterProvider_Spi() {
AutoConfiguredOpenTelemetrySdk.builder()
.setResultAsGlobal(false)
.addPropertiesSupplier(
() -> {
return ImmutableMap.of(
"otel.traces.exporter",
"none",
"otel.metrics.exporter",
"none",
"otel.experimental.metrics.view.config",
"classpath:/view-config-customizer-test.yaml");
})
() ->
ImmutableMap.of(
"otel.traces.exporter",
"none",
"otel.metrics.exporter",
"none",
"otel.experimental.metrics.view.config",
"classpath:/view-config-customizer-test.yaml"))
.addMeterProviderCustomizer(
(meterProviderBuilder, configProperties) ->
meterProviderBuilder.registerMetricReader(reader))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ void loadViewConfig_FullConfig() {
SelectorSpecification selectorSpec = viewConfigSpec.getSelectorSpecification();
assertThat(selectorSpec.getInstrumentName()).isEqualTo("name1");
assertThat(selectorSpec.getInstrumentType()).isEqualTo(InstrumentType.COUNTER);
assertThat(selectorSpec.getInstrumentUnit()).isEqualTo("ms");
assertThat(selectorSpec.getMeterName()).isEqualTo("meterName1");
assertThat(selectorSpec.getMeterVersion()).isEqualTo("1.0.0");
assertThat(selectorSpec.getMeterSchemaUrl()).isEqualTo("http://example1.com");
Expand All @@ -72,6 +73,7 @@ void loadViewConfig_FullConfig() {
SelectorSpecification selectorSpec = viewConfigSpec.getSelectorSpecification();
assertThat(selectorSpec.getInstrumentName()).isEqualTo("name2");
assertThat(selectorSpec.getInstrumentType()).isEqualTo(InstrumentType.COUNTER);
assertThat(selectorSpec.getInstrumentUnit()).isEqualTo("s");
assertThat(selectorSpec.getMeterName()).isEqualTo("meterName2");
assertThat(selectorSpec.getMeterVersion()).isEqualTo("2.0.0");
assertThat(selectorSpec.getMeterSchemaUrl()).isEqualTo("http://example2.com");
Expand Down Expand Up @@ -153,22 +155,22 @@ void toView() {
.extracting(
"attributesProcessor", as(InstanceOfAssertFactories.type(AttributesProcessor.class)))
.satisfies(
attributesProcessor -> {
assertThat(
attributesProcessor.process(
Attributes.builder()
.put("foo", "val")
.put("bar", "val")
.put("baz", "val")
.build(),
Context.current()))
.containsEntry("foo", "val")
.containsEntry("bar", "val")
.satisfies(
(Consumer<Attributes>)
attributes ->
assertThat(attributes.get(AttributeKey.stringKey("baz"))).isBlank());
});
attributesProcessor ->
assertThat(
attributesProcessor.process(
Attributes.builder()
.put("foo", "val")
.put("bar", "val")
.put("baz", "val")
.build(),
Context.current()))
.containsEntry("foo", "val")
.containsEntry("bar", "val")
.satisfies(
(Consumer<Attributes>)
attributes ->
assertThat(attributes.get(AttributeKey.stringKey("baz")))
.isBlank()));
}

@Test
Expand Down Expand Up @@ -242,13 +244,15 @@ void toInstrumentSelector() {
SelectorSpecification.builder()
.instrumentName("name")
.instrumentType(InstrumentType.COUNTER)
.instrumentUnit("ms")
.meterName("meterName")
.meterVersion("meterVersion")
.meterSchemaUrl("http://example.com")
.build());

assertThat(selector.getInstrumentName()).isEqualTo("name");
assertThat(selector.getInstrumentType()).isEqualTo(InstrumentType.COUNTER);
assertThat(selector.getInstrumentUnit()).isEqualTo("ms");
assertThat(selector.getMeterName()).isEqualTo("meterName");
assertThat(selector.getMeterVersion()).isEqualTo("meterVersion");
assertThat(selector.getMeterSchemaUrl()).isEqualTo("http://example.com");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
- selector:
instrument_name: name1
instrument_type: COUNTER
instrument_unit: ms
meter_name: meterName1
meter_version: 1.0.0
meter_schema_url: http://example1.com
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
- selector:
instrument_name: name1
instrument_type: COUNTER
instrument_unit: ms
meter_name: meterName1
meter_version: 1.0.0
meter_schema_url: http://example1.com
Expand All @@ -14,6 +15,7 @@
- selector:
instrument_name: name2
instrument_type: COUNTER
instrument_unit: s
meter_name: meterName2
meter_version: 2.0.0
meter_schema_url: http://example2.com
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@ public static InstrumentSelectorBuilder builder() {
static InstrumentSelector create(
@Nullable InstrumentType instrumentType,
@Nullable String instrumentName,
@Nullable String instrumentUnit,
@Nullable String meterName,
@Nullable String meterVersion,
@Nullable String meterSchemaUrl) {
return new AutoValue_InstrumentSelector(
instrumentType, instrumentName, meterName, meterVersion, meterSchemaUrl);
instrumentType, instrumentName, instrumentUnit, meterName, meterVersion, meterSchemaUrl);
}

InstrumentSelector() {}
Expand All @@ -59,6 +60,10 @@ static InstrumentSelector create(
@Nullable
public abstract String getInstrumentName();

/** Returns the selected instrument unit, or null if this selects all instrument units. */
@Nullable
public abstract String getInstrumentUnit();

/** Returns the selected meter name, or null if this selects instruments from all meter names. */
@Nullable
public abstract String getMeterName();
Expand Down Expand Up @@ -86,6 +91,9 @@ public final String toString() {
if (getInstrumentName() != null) {
joiner.add("instrumentName=" + getInstrumentName());
}
if (getInstrumentUnit() != null) {
joiner.add("instrumentUnit=" + getInstrumentUnit());
}
if (getMeterName() != null) {
joiner.add("meterName=" + getMeterName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public final class InstrumentSelectorBuilder {

@Nullable private InstrumentType instrumentType;
@Nullable private String instrumentName;
@Nullable private String instrumentUnit;
@Nullable private String meterName;
@Nullable private String meterVersion;
@Nullable private String meterSchemaUrl;
Expand Down Expand Up @@ -49,6 +50,13 @@ public InstrumentSelectorBuilder setName(String name) {
return this;
}

/** Select instruments with the given {@code unit}. */
public InstrumentSelectorBuilder setUnit(String unit) {
requireNonNull(unit, "unit");
this.instrumentUnit = unit;
return this;
}

/** Select instruments associated with the given {@code meterName}. */
public InstrumentSelectorBuilder setMeterName(String meterName) {
requireNonNull(meterName, "meterName");
Expand All @@ -75,11 +83,12 @@ public InstrumentSelector build() {
checkArgument(
instrumentType != null
|| instrumentName != null
|| instrumentUnit != null
|| meterName != null
|| meterVersion != null
|| meterSchemaUrl != null,
"Instrument selector must contain selection criteria");
return InstrumentSelector.create(
instrumentType, instrumentName, meterName, meterVersion, meterSchemaUrl);
instrumentType, instrumentName, instrumentUnit, meterName, meterVersion, meterSchemaUrl);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ private static boolean matchesSelector(
&& selector.getInstrumentType() != descriptor.getType()) {
return false;
}
if (selector.getInstrumentUnit() != null
&& !selector.getInstrumentUnit().equals(descriptor.getUnit())) {
return false;
}
if (selector.getInstrumentName() != null
&& !toGlobPatternPredicate(selector.getInstrumentName()).test(descriptor.getName())) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ void invalidArgs() {
assertThatThrownBy(() -> InstrumentSelector.builder().setName(null))
.isInstanceOf(NullPointerException.class)
.hasMessage("name");
assertThatThrownBy(() -> InstrumentSelector.builder().setUnit(null))
.isInstanceOf(NullPointerException.class)
.hasMessage("unit");
assertThatThrownBy(() -> InstrumentSelector.builder().setMeterName(null))
.isInstanceOf(NullPointerException.class)
.hasMessage("meterName");
Expand Down Expand Up @@ -51,6 +54,12 @@ void instrumentName() {
assertThat(selector.getInstrumentName()).isEqualTo("bar");
}

@Test
void instrumentUnit() {
InstrumentSelector selector = InstrumentSelector.builder().setName("ms").setName("s").build();
assertThat(selector.getInstrumentName()).isEqualTo("s");
}

@Test
void meterName() {
InstrumentSelector selector =
Expand Down Expand Up @@ -83,6 +92,7 @@ void stringRepresentation() {
InstrumentSelector.builder()
.setType(InstrumentType.COUNTER)
.setName("name")
.setUnit("unit")
.setMeterName("meter")
.setMeterVersion("version")
.setMeterSchemaUrl("http://url.com")
Expand All @@ -92,6 +102,7 @@ void stringRepresentation() {
"InstrumentSelector{"
+ "instrumentType=COUNTER, "
+ "instrumentName=name, "
+ "instrumentUnit=unit, "
+ "meterName=meter, "
+ "meterVersion=version, "
+ "meterSchemaUrl=http://url.com"
Expand Down
Loading