-
Notifications
You must be signed in to change notification settings - Fork 27
Support for handling LLO in OTLP AWS Logs Exporter #364
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
Support for handling LLO in OTLP AWS Logs Exporter #364
Conversation
|
Thanks! Looks good overall. Couple of small things:
|
| @@ -0,0 +1,4 @@ | |||
| BASE_LOG_BUFFER_BYTE_SIZE = 2000 | |||
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.
Where does this number come from?
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.
I ran an e2e test with the Gen AI sample app and subtracted the size of the body content from the size of the entire log itself. I then multiplied by 3 and added a bit of extra buffer to account for larger logs.
| if isinstance(val, Sequence): | ||
| for content in val: | ||
| size += AwsBatchLogRecordProcessor._get_size_of_any_value(content) | ||
|
|
||
| if isinstance(val, Mapping): | ||
| for _, content in val.items(): | ||
| size += AwsBatchLogRecordProcessor._get_size_of_any_value(content) |
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.
Do we need these? The Gen AI Events we are emitting from the Span pipeline only contain primitive data types.
This recursive approach traverses the entire object graph depth-first, which could become expensive for:
- Deeply nested JSON structures
- Large arrays of complex objects
- Circular references (would cause infinite recursion if present)
- Dictionary keys aren't counted in the size calculation (potential underestimation)
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.
This processor will have to account for more than just gen ai events. All logs emitted by the application will be processed through this so we have to account for more than primitive types.
Supporting ADOT auto instrumentation to handle LLO log events.
Description of changes:
AwsBatchLogRecordProcessora backwards compatible custom logs BatchProcessor which has the following invariants:exported batch is ever > 1 MBthen the batch size is always length 1OTLPAwsLogExporter: Adds a new behavior for Retry delay based on server-side response of Retry-After header. Injects the LLO header flag if the size of the exported data > 1 MB.AwsBatchLogRecordProcessorTesting:
TODO:
AwsBatchLogRecordProcessorandOTLPAwsLogExporterBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.