Skip to content

Unify aws lambda flush handling #12576

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 6, 2024
Merged

Unify aws lambda flush handling #12576

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2029b37
Unify aws lambda flush handling
laurit Nov 6, 2024
4e79015
fix typo
laurit Nov 6, 2024
2c854a8
add back original method
laurit Nov 6, 2024
2b33e5c
Merge remote-tracking branch 'upstream/main' into lambda-flush
trask Nov 6, 2024
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
5 changes: 5 additions & 0 deletions instrumentation/aws-lambda/aws-lambda-core-1.0/javaagent/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Settings for the AWS Lambda Instrumentation

| System property | Type | Default | Description |
|-------------------------------------------------|---------|---------|--------------------------------|
| `otel.instrumentation.aws-lambda.flush-timeout` | Integer | 10000 | Flush timeout in milliseconds. |
Copy link
Contributor

@steverao steverao Nov 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using Long instead of Integer here? in java.util.concurrent.TimeUnit, I noticed it used long type for millisecond or other unit of time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it really matter? From user perspective I'd assume it is only important to know that a numeric value is expected.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for configuration, I think we only need to specify something like "int" or "double", similar to semantic convention attribute types

13 changes: 13 additions & 0 deletions ...elemetry/javaagent/instrumentation/awslambdacore/v1_0/AwsLambdaInstrumentationHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,28 @@
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.instrumentation.awslambdacore.v1_0.internal.AwsLambdaFunctionInstrumenter;
import io.opentelemetry.instrumentation.awslambdacore.v1_0.internal.AwsLambdaFunctionInstrumenterFactory;
import io.opentelemetry.instrumentation.awslambdacore.v1_0.internal.WrapperConfiguration;
import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig;
import java.time.Duration;

public final class AwsLambdaInstrumentationHelper {

private static final AwsLambdaFunctionInstrumenter FUNCTION_INSTRUMENTER =
AwsLambdaFunctionInstrumenterFactory.createInstrumenter(GlobalOpenTelemetry.get());
private static final Duration FLUSH_TIMEOUT =
Duration.ofMillis(
AgentInstrumentationConfig.get()
.getLong(
"otel.instrumentation.aws-lambda.flush-timeout",
WrapperConfiguration.OTEL_LAMBDA_FLUSH_TIMEOUT_DEFAULT.toMillis()));

public static AwsLambdaFunctionInstrumenter functionInstrumenter() {
return FUNCTION_INSTRUMENTER;
}

public static Duration flushTimeout() {
return FLUSH_TIMEOUT;
}

private AwsLambdaInstrumentationHelper() {}
}
3 changes: 2 additions & 1 deletion .../javaagent/instrumentation/awslambdacore/v1_0/AwsLambdaRequestHandlerInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
import static io.opentelemetry.javaagent.instrumentation.awslambdacore.v1_0.AwsLambdaInstrumentationHelper.flushTimeout;
import static io.opentelemetry.javaagent.instrumentation.awslambdacore.v1_0.AwsLambdaInstrumentationHelper.functionInstrumenter;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
Expand Down Expand Up @@ -90,7 +91,7 @@ public static void stopSpan(
functionInstrumenter().end(functionContext, input, null, throwable);
}

OpenTelemetrySdkAccess.forceFlush(1, TimeUnit.SECONDS);
OpenTelemetrySdkAccess.forceFlush(flushTimeout().toNanos(), TimeUnit.NANOSECONDS);
}
}
}
2 changes: 1 addition & 1 deletion instrumentation/aws-lambda/aws-lambda-core-1.0/library/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ This package contains libraries to help instrument AWS lambda functions in your
To use the instrumentation, configure `OTEL_INSTRUMENTATION_AWS_LAMBDA_HANDLER` env property to your lambda handler method in following format `package.ClassName::methodName`
and use one of wrappers as your lambda `Handler`.

In order to configure a span flush timeout (default is set to 1 second), please configure `OTEL_INSTRUMENTATION_AWS_LAMBDA_FLUSH_TIMEOUT` env property. The value is in seconds.
In order to configure a span flush timeout (default is set to 10 second), please configure `OTEL_INSTRUMENTATION_AWS_LAMBDA_FLUSH_TIMEOUT` env property. The value is in milliseconds.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell this is configured in

opentelemetry-java-instrumentation/instrumentation/aws-lambda/aws-lambda-core-1.0/library/src/main/java/io/opentelemetry/instrumentation/awslambdacore/v1_0/internal/WrapperConfiguration.java

Lines 20 to 32 in 78b3c6a

public static final Duration OTEL_LAMBDA_FLUSH_TIMEOUT_DEFAULT = Duration.ofSeconds(10);
public static Duration flushTimeout() {
String lambdaFlushTimeout = System.getenv(OTEL_LAMBDA_FLUSH_TIMEOUT_ENV_KEY);
if (lambdaFlushTimeout != null && !lambdaFlushTimeout.isEmpty()) {
try {
return Duration.ofMillis(Long.parseLong(lambdaFlushTimeout));
} catch (NumberFormatException nfe) {
// ignored - default used
}
}
return OTEL_LAMBDA_FLUSH_TIMEOUT_DEFAULT;
}


Available wrappers:

Expand Down
5 changes: 5 additions & 0 deletions instrumentation/aws-lambda/aws-lambda-events-2.2/javaagent/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Settings for the AWS Lambda Instrumentation

| System property | Type | Default | Description |
|-------------------------------------------------|---------|---------|--------------------------------|
| `otel.instrumentation.aws-lambda.flush-timeout` | Integer | 10000 | Flush timeout in milliseconds. |
33 changes: 21 additions & 12 deletions ...emetry/javaagent/instrumentation/awslambdaevents/v2_2/AwsLambdaInstrumentationHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,39 @@
import com.amazonaws.services.lambda.runtime.events.SQSEvent;
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.awslambdacore.v1_0.internal.AwsLambdaFunctionInstrumenter;
import io.opentelemetry.instrumentation.awslambdacore.v1_0.internal.WrapperConfiguration;
import io.opentelemetry.instrumentation.awslambdaevents.v2_2.internal.AwsLambdaEventsInstrumenterFactory;
import io.opentelemetry.instrumentation.awslambdaevents.v2_2.internal.AwsLambdaSqsInstrumenterFactory;
import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig;
import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig;
import java.time.Duration;

public final class AwsLambdaInstrumentationHelper {

private static final io.opentelemetry.instrumentation.awslambdacore.v1_0.internal
.AwsLambdaFunctionInstrumenter
FUNCTION_INSTRUMENTER =
AwsLambdaEventsInstrumenterFactory.createInstrumenter(
GlobalOpenTelemetry.get(), AgentCommonConfig.get().getKnownHttpRequestMethods());

public static io.opentelemetry.instrumentation.awslambdacore.v1_0.internal
.AwsLambdaFunctionInstrumenter
functionInstrumenter() {
return FUNCTION_INSTRUMENTER;
}

private static final AwsLambdaFunctionInstrumenter FUNCTION_INSTRUMENTER =
AwsLambdaEventsInstrumenterFactory.createInstrumenter(
GlobalOpenTelemetry.get(), AgentCommonConfig.get().getKnownHttpRequestMethods());
private static final Instrumenter<SQSEvent, Void> MESSAGE_TRACER =
AwsLambdaSqsInstrumenterFactory.forEvent(GlobalOpenTelemetry.get());
private static final Duration FLUSH_TIMEOUT =
Duration.ofMillis(
AgentInstrumentationConfig.get()
.getLong(
"otel.instrumentation.aws-lambda.flush-timeout",
WrapperConfiguration.OTEL_LAMBDA_FLUSH_TIMEOUT_DEFAULT.toMillis()));

public static AwsLambdaFunctionInstrumenter functionInstrumenter() {
return FUNCTION_INSTRUMENTER;
}

public static Instrumenter<SQSEvent, Void> messageInstrumenter() {
return MESSAGE_TRACER;
}

public static Duration flushTimeout() {
return FLUSH_TIMEOUT;
}

private AwsLambdaInstrumentationHelper() {}
}
3 changes: 2 additions & 1 deletion ...avaagent/instrumentation/awslambdaevents/v2_2/AwsLambdaRequestHandlerInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
import static io.opentelemetry.javaagent.instrumentation.awslambdaevents.v2_2.AwsLambdaInstrumentationHelper.flushTimeout;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
import static net.bytebuddy.matcher.ElementMatchers.named;
Expand Down Expand Up @@ -114,7 +115,7 @@ public static void stopSpan(
.end(functionContext, input, result, throwable);
}

OpenTelemetrySdkAccess.forceFlush(1, TimeUnit.SECONDS);
OpenTelemetrySdkAccess.forceFlush(flushTimeout().toNanos(), TimeUnit.NANOSECONDS);
}
}
}
2 changes: 1 addition & 1 deletion instrumentation/aws-lambda/aws-lambda-events-2.2/library/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ This package contains libraries to help instrument AWS lambda functions in your
To use the instrumentation, configure `OTEL_INSTRUMENTATION_AWS_LAMBDA_HANDLER` env property to your lambda handler method in following format `package.ClassName::methodName`
and use one of wrappers as your lambda `Handler`.

In order to configure a span flush timeout (default is set to 1 second), please configure `OTEL_INSTRUMENTATION_AWS_LAMBDA_FLUSH_TIMEOUT` env property. The value is in seconds.
In order to configure a span flush timeout (default is set to 10 second), please configure `OTEL_INSTRUMENTATION_AWS_LAMBDA_FLUSH_TIMEOUT` env property. The value is in milliseconds.

Available wrappers:

Expand Down
4 changes: 2 additions & 2 deletions ...-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/OpenTelemetrySdkAccess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ public final class OpenTelemetrySdkAccess {
*/
public interface ForceFlusher {
/** Executes force flush. */
void run(int timeout, TimeUnit unit);
void run(long timeout, TimeUnit unit);
}

private static volatile ForceFlusher forceFlush;

/** Forces flushing of pending spans. */
public static void forceFlush(int timeout, TimeUnit unit) {
public static void forceFlush(long timeout, TimeUnit unit) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe keep and mark the int variant deprecated for removal in 3.0? seems like something that could be in use by an extension

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added back the original method. If the indy extension changes get done in time for 3.0 it is possible that we won't need most of the bootstrap classes any more.

forceFlush.run(timeout, unit);
}

Expand Down
Loading