auditing - initial changes #280
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ahmarsuhail
had a problem deploying
to
integration-tests
GitHub Actions
Failure
4 tasks
rajdchak
reviewed
Jun 3, 2025
| public class StreamContext { | ||
| private final String spanId; | ||
| private final String operationName; | ||
| } |
Contributor
There was a problem hiding this comment.
should we be removing the referrer header builder support?
ahmarsuhail
had a problem deploying
to
integration-tests
GitHub Actions
Failure
ahmarsuhail
had a problem deploying
to
integration-tests
GitHub Actions
Failure
ahmarsuhail
had a problem deploying
to
integration-tests
GitHub Actions
Failure
rajdchak
reviewed
Jun 4, 2025
| final String referrerHeader; | ||
| AwsRequestOverrideConfiguration.Builder requestOverrideConfigurationBuilder = | ||
| AwsRequestOverrideConfiguration.builder() | ||
| .putHeader(HEADER_REFERER, getRequest.getReferrer().toString()) |
Contributor
There was a problem hiding this comment.
can we have all the request related constants like HEADER_REFERER and HEADER_USER_AGENT also as part of the request attributes class?
rajdchak
reviewed
Jun 4, 2025
| public String modifyAndBuildReferrerHeader(GetRequest getRequestContext); | ||
| @Builder(access = AccessLevel.PUBLIC) | ||
| @Getter | ||
| public class StreamContext { |
Contributor
There was a problem hiding this comment.
Nit: Should we rename this class to better reflect its auditing-specific purpose? like StreamAuditingContext
ahmarsuhail
had a problem deploying
to
integration-tests
GitHub Actions
Failure
ahmarsuhail
had a problem deploying
to
integration-tests
GitHub Actions
Failure
ahmarsuhail
temporarily deployed
to
integration-tests
GitHub Actions
Inactive
rajdchak
approved these changes
Jun 5, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of change
S3A PR: apache/hadoop#7723
Use executionAttributes to attach the correct span to each get request made for a stream.
S3A generates a
span_idfor each operation. For example, when a stream to an object is opened in S3A'sexecuteOpen(), a span_id is generated for this operation. TheexecuteOpen()operation is also given the nameop_open.This
span_idis then used to log any S3 requests that happened for the operation in S3A's LoggingAuditor. For example, for anexecuteOpen(), there can be a HEAD request, and a GET request, and these show up in the logs as:The
84ca7693-a471-4564-a6b9-70b6424548c4-00000009is the span-id. From the above logs, we can decipher exactly what S3 requests were made per operation, which helps in debugging.Since the S3 GETs are now made in AAL, we must find a way to attach these back to the correct span and operation. This is done as follows:
span_idandoperation_nameof the stream. Thisspan_idis generated at stream creation time.ExecutionAttributes.S3A's LoggingAuditor will then be able to access these in it's ExecutionInterceptor, and use it for it's logging and building the referrer header.
Relevant issues
Does this contribution introduce any breaking changes to the existing APIs or behaviors?
Does this contribution introduce any new public APIs or behaviors?
How was the contribution tested?
Does this contribution need a changelog entry?
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).