-
Notifications
You must be signed in to change notification settings - Fork 892
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
Unify aws lambda flush handling #12576
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. | | ||
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. | |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,13 +22,19 @@ 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. */ | ||
/** Forces flushing of pending telemetry. */ | ||
@Deprecated | ||
public static void forceFlush(int timeout, TimeUnit unit) { | ||
forceFlush((long) timeout, unit); | ||
} | ||
|
||
/** Forces flushing of pending telemetry. */ | ||
public static void forceFlush(long timeout, TimeUnit unit) { | ||
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. maybe keep and mark the 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. 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); | ||
} | ||
|
||
|
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.
How about using
Long
instead ofInteger
here? injava.util.concurrent.TimeUnit
, I noticed it used long type for millisecond or other unit of time.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.
Does it really matter? From user perspective I'd assume it is only important to know that a numeric value is expected.
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.
for configuration, I think we only need to specify something like "int" or "double", similar to semantic convention attribute types