Skip to content

Commit

Permalink
Fix ingestion sampling (#3578)
Browse files Browse the repository at this point in the history
  • Loading branch information
trask authored Mar 7, 2024
1 parent 9306596 commit 9f9598f
Show file tree
Hide file tree
Showing 44 changed files with 249 additions and 214 deletions.
2 changes: 1 addition & 1 deletion agent/agent-tooling/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dependencies {
implementation(project(":agent:agent-profiler:agent-diagnostics"))
implementation(project(":etw:java"))

implementation("com.azure:azure-monitor-opentelemetry-exporter:1.0.0-beta.19")
implementation("com.azure:azure-monitor-opentelemetry-exporter:1.0.0-beta.20")
compileOnly("io.opentelemetry.javaagent:opentelemetry-javaagent-bootstrap")
compileOnly("io.opentelemetry.javaagent:opentelemetry-javaagent-tooling")
compileOnly("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-servlet-common-bootstrap")
Expand Down
2 changes: 1 addition & 1 deletion agent/agent-tooling/gradle.lockfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ com.azure:azure-core-http-netty:1.14.0=runtimeClasspath
com.azure:azure-core:1.46.0=runtimeClasspath
com.azure:azure-identity:1.11.3=runtimeClasspath
com.azure:azure-json:1.1.0=runtimeClasspath
com.azure:azure-monitor-opentelemetry-exporter:1.0.0-beta.19=runtimeClasspath
com.azure:azure-monitor-opentelemetry-exporter:1.0.0-beta.20=runtimeClasspath
com.azure:azure-sdk-bom:1.2.21=runtimeClasspath
com.azure:azure-storage-blob:12.25.2=runtimeClasspath
com.azure:azure-storage-common:12.24.2=runtimeClasspath
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public class BytecodeUtilImpl implements BytecodeUtilDelegate {
private static final AtomicBoolean alreadyLoggedError = new AtomicBoolean();

// in Azure Functions consumption pool, we don't know at startup whether to enable or not
// TODO (trask) convert this from float to double?
public static volatile float samplingPercentage = 0;

public static volatile FeatureStatsbeat featureStatsbeat;
Expand Down Expand Up @@ -505,8 +506,10 @@ private static void track(
}

if (isPartOfTheCurrentTrace && applySampling && span instanceof ReadableSpan) {
long itemCount = getItemCount((ReadableSpan) span);
telemetryBuilder.setSampleRate(100.0f / itemCount);
Long itemCount = ((ReadableSpan) span).getAttribute(AiSemanticAttributes.ITEM_COUNT);
if (itemCount != null) {
telemetryBuilder.setSampleRate(100.0f / itemCount);
}
}

if (!isPartOfTheCurrentTrace && applySampling) {
Expand All @@ -519,7 +522,9 @@ private static void track(
}
// sampled in

telemetryBuilder.setSampleRate(samplingPercentage);
if (samplingPercentage != 100) {
telemetryBuilder.setSampleRate(samplingPercentage);
}
}

// this is not null because sdk instrumentation is not added until TelemetryClient.setActive()
Expand Down Expand Up @@ -574,11 +579,6 @@ private static boolean sample(String operationId, double samplingPercentage) {
return SamplingScoreGeneratorV2.getSamplingScore(operationId) < samplingPercentage;
}

private static long getItemCount(ReadableSpan span) {
Long itemCount = span.getAttribute(AiSemanticAttributes.ITEM_COUNT);
return itemCount == null ? 1L : itemCount;
}

private static void selectivelySetTags(
AbstractTelemetryBuilder telemetryBuilder, Map<String, String> sourceTags) {
for (Map.Entry<String, String> entry : sourceTags.entrySet()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ private static boolean isEmpty(@Nullable String str) {
public void validate() {
instrumentation.logging.getSeverityThreshold();
authentication.validate();
sampling.validate();
preview.validate();
}

Expand Down Expand Up @@ -168,6 +169,12 @@ public static class Sampling {
@Deprecated @Nullable public Double limitPerSecond;

public List<SamplingOverride> overrides = new ArrayList<>();

private void validate() {
for (SamplingOverride samplingOverride : overrides) {
samplingOverride.validate();
}
}
}

public static class SamplingPreview {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,13 @@ private static void logConfigurationWarnings(Configuration config) {
configurationLogger.warn(
"\"clientsecret\" typed of AAD authentication has been deprecated since 3.5.0 GA. Please use \"user-assigned identity\" or \"system-assigned identity\" instead.");
}
if (config.sampling.percentage != null && config.sampling.requestsPerSecond != null) {
configurationLogger.warn(
"Sampling \"requestsPerSecond\" and \"percentage\" should not be used at the same time."
+ " Please remove one of them.");
config.sampling.percentage = null; // requestsPerSecond takes priority
}

logWarningIfUsingInternalAttributes(config);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ public SamplingResult shouldSample(
: parentlessDependencySamplingPercentage.get();
}

if (sp == SamplingPercentage.USE_INGESTION_SAMPLING) {
return SamplingResult.recordAndSample();
}

if (sp == 0) {
return SamplingResult.drop();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ public static Sampler getSampler(
SamplingPercentage parentlessDependencySamplingPercentage = SamplingPercentage.fixed(100);
sampler = new AiSampler(requestSamplingPercentage, parentlessDependencySamplingPercentage);
} else if (sampling.percentage != null) {
SamplingPercentage samplingPercentage = SamplingPercentage.fixed(sampling.percentage);
SamplingPercentage samplingPercentage;
if (sampling.percentage == 100) {
samplingPercentage = SamplingPercentage.useIngestionSampling();
} else {
samplingPercentage = SamplingPercentage.fixed(sampling.percentage);
}
sampler = new AiSampler(samplingPercentage, samplingPercentage);
} else {
throw new AssertionError("ConfigurationBuilder should have set the default sampling");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
// the portal
public interface SamplingPercentage {

long USE_INGESTION_SAMPLING = -1;

double get();

static SamplingPercentage fixed(double percentage) {
Expand All @@ -22,4 +24,8 @@ static SamplingPercentage fixed(double percentage) {
static SamplingPercentage rateLimited(double targetPerSecondLimit) {
return new RateLimitedSamplingPercentage(targetPerSecondLimit, 0.1);
}

static SamplingPercentage useIngestionSampling() {
return () -> USE_INGESTION_SAMPLING;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@
import io.opentelemetry.sdk.trace.samplers.Sampler;
import io.opentelemetry.sdk.trace.samplers.SamplingResult;
import java.util.Collections;
import javax.annotation.Nullable;

public class SamplingTestUtil {

public static double getCurrentSamplingPercentage(Sampler sampler) {
@Nullable
public static Double getCurrentSamplingPercentage(Sampler sampler) {
SpanContext spanContext =
SpanContext.createFromRemoteParent(
"12341234123412341234123412341234",
Expand All @@ -36,7 +38,7 @@ public static double getCurrentSamplingPercentage(Sampler sampler) {
Attributes.empty(),
Collections.emptyList());
Long itemCount = samplingResult.getAttributes().get(AiSemanticAttributes.ITEM_COUNT);
return 100.0 / itemCount;
return itemCount == null ? null : 100.0 / itemCount;
}

private SamplingTestUtil() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.microsoft.applicationinsights.agent.internal.telemetry.TelemetryClient;
import java.net.URISyntaxException;
import java.nio.file.Paths;
import javax.annotation.Nullable;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -50,17 +51,17 @@ void afterEach() {
void shouldUpdate() throws URISyntaxException {
// given
Configuration config = new Configuration();
config.sampling.percentage = 100.0;
config.sampling.percentage = 50.0;

BytecodeUtilImpl.samplingPercentage = 100;
BytecodeUtilImpl.samplingPercentage = 50;

TelemetryClient telemetryClient = TelemetryClient.createForTest();
telemetryClient.updateConnectionStrings(
"InstrumentationKey=00000000-0000-0000-0000-000000000000", null, null);

RpConfiguration rpConfiguration = new RpConfiguration();
rpConfiguration.connectionString = "InstrumentationKey=11111111-1111-1111-1111-111111111111";
rpConfiguration.sampling.percentage = 90.0;
rpConfiguration.connectionString = "InstrumentationKey=00000000-0000-0000-0000-000000000000";
rpConfiguration.sampling.percentage = 50.0;
rpConfiguration.configPath =
Paths.get(
RpConfigurationPollingTest.class.getResource("/applicationinsights-rp.json").toURI());
Expand All @@ -69,8 +70,8 @@ void shouldUpdate() throws URISyntaxException {
// pre-check
assertThat(telemetryClient.getInstrumentationKey())
.isEqualTo("00000000-0000-0000-0000-000000000000");
assertThat(BytecodeUtilImpl.samplingPercentage).isEqualTo(100);
assertThat(getCurrentSamplingPercentage()).isEqualTo(100);
assertThat(BytecodeUtilImpl.samplingPercentage).isEqualTo(50);
assertThat(getCurrentSamplingPercentage()).isNull();

// when
RuntimeConfigurator runtimeConfigurator =
Expand All @@ -79,41 +80,42 @@ void shouldUpdate() throws URISyntaxException {

// then
assertThat(telemetryClient.getInstrumentationKey())
.isEqualTo("00000000-0000-0000-0000-000000000000");
assertThat(BytecodeUtilImpl.samplingPercentage).isEqualTo(10);
.isEqualTo("11111111-1111-1111-1111-111111111111");
// TODO (trask) fix, this should really be 10
assertThat(BytecodeUtilImpl.samplingPercentage).isEqualTo(9.9f);
assertThat(getCurrentSamplingPercentage()).isEqualTo(10);
}

@Test
void shouldBePopulatedByEnvVars() throws URISyntaxException {
// given
Configuration config = new Configuration();
config.sampling.percentage = 100.0;
config.sampling.percentage = 50.0;

BytecodeUtilImpl.samplingPercentage = 100;
BytecodeUtilImpl.samplingPercentage = 50;

TelemetryClient telemetryClient = TelemetryClient.createForTest();
telemetryClient.updateConnectionStrings(
"InstrumentationKey=00000000-0000-0000-0000-000000000000", null, null);

RpConfiguration rpConfiguration = new RpConfiguration();
rpConfiguration.connectionString = "InstrumentationKey=11111111-1111-1111-1111-111111111111";
rpConfiguration.sampling.percentage = 90.0;
rpConfiguration.connectionString = "InstrumentationKey=00000000-0000-0000-0000-000000000000";
rpConfiguration.sampling.percentage = 50.0;
rpConfiguration.configPath =
Paths.get(
RpConfigurationPollingTest.class.getResource("/applicationinsights-rp.json").toURI());
rpConfiguration.lastModifiedTime = 0;

envVars.set(
"APPLICATIONINSIGHTS_CONNECTION_STRING",
"InstrumentationKey=00000000-0000-0000-0000-000000000000");
envVars.set("APPLICATIONINSIGHTS_SAMPLING_PERCENTAGE", "90");
"InstrumentationKey=22222222-2222-2222-2222-222222222222");
envVars.set("APPLICATIONINSIGHTS_SAMPLING_PERCENTAGE", "24");

// pre-check
assertThat(telemetryClient.getInstrumentationKey())
.isEqualTo("00000000-0000-0000-0000-000000000000");
assertThat(BytecodeUtilImpl.samplingPercentage).isEqualTo(100);
assertThat(getCurrentSamplingPercentage()).isEqualTo(100);
assertThat(BytecodeUtilImpl.samplingPercentage).isEqualTo(50);
assertThat(getCurrentSamplingPercentage()).isNull();

// when
RuntimeConfigurator runtimeConfigurator =
Expand All @@ -122,12 +124,14 @@ void shouldBePopulatedByEnvVars() throws URISyntaxException {

// then
assertThat(telemetryClient.getInstrumentationKey())
.isEqualTo("00000000-0000-0000-0000-000000000000");
assertThat(BytecodeUtilImpl.samplingPercentage).isEqualTo(100);
assertThat(getCurrentSamplingPercentage()).isEqualTo(100);
.isEqualTo("22222222-2222-2222-2222-222222222222");
// TODO (trask) fix, this should really be 24
assertThat(BytecodeUtilImpl.samplingPercentage).isEqualTo(24);
assertThat(getCurrentSamplingPercentage()).isEqualTo(25);
}

private static double getCurrentSamplingPercentage() {
@Nullable
private static Double getCurrentSamplingPercentage() {
return SamplingTestUtil.getCurrentSamplingPercentage(DelegatingSampler.getInstance());
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"connectionString": "InstrumentationKey=00000000-0000-0000-0000-000000000000",
"connectionString": "InstrumentationKey=11111111-1111-1111-1111-111111111111",
"sampling": {
"percentage": 10
"percentage": 9.9
}
}
2 changes: 1 addition & 1 deletion licenses/more-licenses.md
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ _2024-03-04 03:40:01 UTC_
> - **POM Project URL**: [https://github.com/Azure/azure-sdk-for-java](https://github.com/Azure/azure-sdk-for-java)
> - **POM License**: MIT License - [https://opensource.org/licenses/MIT](https://opensource.org/licenses/MIT)
**68** **Group:** `com.azure` **Name:** `azure-monitor-opentelemetry-exporter` **Version:** `1.0.0-beta.19`
**68** **Group:** `com.azure` **Name:** `azure-monitor-opentelemetry-exporter` **Version:** `1.0.0-beta.20`
> - **POM License**: MIT License - [https://opensource.org/licenses/MIT](https://opensource.org/licenses/MIT)
**69** **Group:** `com.azure` **Name:** `azure-storage-blob` **Version:** `12.25.2`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ void test() throws Exception {
Envelope mdEnvelope2 = mdList.get(1);
Envelope mdEnvelope3 = mdList.get(2);

assertThat(rdEnvelope.getSampleRate()).isEqualTo(100.0f);
assertThat(mdEnvelope1.getSampleRate()).isEqualTo(100.0f);
assertThat(mdEnvelope2.getSampleRate()).isEqualTo(100.0f);
assertThat(mdEnvelope3.getSampleRate()).isEqualTo(100.0f);
assertThat(rdEnvelope.getSampleRate()).isNull();
assertThat(mdEnvelope1.getSampleRate()).isNull();
assertThat(mdEnvelope2.getSampleRate()).isNull();
assertThat(mdEnvelope3.getSampleRate()).isNull();

RequestData rd = (RequestData) ((Data<?>) rdEnvelope.getData()).getBaseData();

Expand Down Expand Up @@ -109,8 +109,8 @@ void testWithException() throws Exception {

Envelope edEnvelope = edList.get(0);

assertThat(rdEnvelope.getSampleRate()).isEqualTo(100.0f);
assertThat(edEnvelope.getSampleRate()).isEqualTo(100.0f);
assertThat(rdEnvelope.getSampleRate()).isNull();
assertThat(edEnvelope.getSampleRate()).isNull();

RequestData rd = (RequestData) ((Data<?>) rdEnvelope.getData()).getBaseData();
ExceptionData ed = (ExceptionData) ((Data<?>) edEnvelope.getData()).getBaseData();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ void test() throws Exception {
Envelope mdEnvelope2 = mdList.get(1);
Envelope mdEnvelope3 = mdList.get(2);

assertThat(rdEnvelope.getSampleRate()).isEqualTo(100.0f);
assertThat(mdEnvelope1.getSampleRate()).isEqualTo(100.0f);
assertThat(mdEnvelope2.getSampleRate()).isEqualTo(100.0f);
assertThat(mdEnvelope3.getSampleRate()).isEqualTo(100.0f);
assertThat(rdEnvelope.getSampleRate()).isNull();
assertThat(mdEnvelope1.getSampleRate()).isNull();
assertThat(mdEnvelope2.getSampleRate()).isNull();
assertThat(mdEnvelope3.getSampleRate()).isNull();

RequestData rd = (RequestData) ((Data<?>) rdEnvelope.getData()).getBaseData();

Expand Down Expand Up @@ -101,8 +101,8 @@ void testWithException() throws Exception {

Envelope edEnvelope = edList.get(0);

assertThat(rdEnvelope.getSampleRate()).isEqualTo(100.0f);
assertThat(edEnvelope.getSampleRate()).isEqualTo(100.0f);
assertThat(rdEnvelope.getSampleRate()).isNull();
assertThat(edEnvelope.getSampleRate()).isNull();

RequestData rd = (RequestData) ((Data<?>) rdEnvelope.getData()).getBaseData();
ExceptionData ed = (ExceptionData) ((Data<?>) edEnvelope.getData()).getBaseData();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ void test() throws Exception {
Envelope mdEnvelope1 = mdList.get(0);
Envelope mdEnvelope2 = mdList.get(1);

assertThat(rdEnvelope.getSampleRate()).isEqualTo(100.0f);
assertThat(mdEnvelope1.getSampleRate()).isEqualTo(100.0f);
assertThat(mdEnvelope2.getSampleRate()).isEqualTo(100.0f);
assertThat(rdEnvelope.getSampleRate()).isNull();
assertThat(mdEnvelope1.getSampleRate()).isNull();
assertThat(mdEnvelope2.getSampleRate()).isNull();

RequestData rd = (RequestData) ((Data<?>) rdEnvelope.getData()).getBaseData();

Expand Down Expand Up @@ -88,8 +88,8 @@ void testWithException() throws Exception {

Envelope edEnvelope = edList.get(0);

assertThat(rdEnvelope.getSampleRate()).isEqualTo(100.0f);
assertThat(edEnvelope.getSampleRate()).isEqualTo(100.0f);
assertThat(rdEnvelope.getSampleRate()).isNull();
assertThat(edEnvelope.getSampleRate()).isNull();

RequestData rd = (RequestData) ((Data<?>) rdEnvelope.getData()).getBaseData();
ExceptionData ed = (ExceptionData) ((Data<?>) edEnvelope.getData()).getBaseData();
Expand Down
Loading

0 comments on commit 9f9598f

Please sign in to comment.