Skip to content
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

Implement AccessLog support for Ballerina #1969

Open
wants to merge 2 commits into
base: 2201.8.x
Choose a base branch
from

Conversation

AzeemMuzammil
Copy link
Member

Purpose

$title. please refer to the below proposal.
ballerina-platform/ballerina-library#6111

Examples

Checklist

  • Linked to an issue
  • Updated the changelog
  • Added tests
  • Updated the spec
  • Checked native-image compatibility
  • Checked the impact on OpenAPI generation

@@ -144,6 +145,9 @@ public synchronized void addHttpContent(HttpContent httpContent) {
public HttpContent getHttpContent() {
HttpContent httpContent = this.blockingEntityCollector.getHttpContent();
this.contentObservable.notifyGetListener(httpContent);
if (httpContent != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

blockingEntityCollector can contain multiple http contents right? In this case we only consider 1 content and set it to the carbon msg. Is that correct?

@@ -53,6 +59,7 @@ public class ReceivingEntityBody implements SenderState {
private final Http2TargetHandler http2TargetHandler;
private final Http2ClientChannel http2ClientChannel;
private final Http2TargetHandler.Http2RequestWriter http2RequestWriter;
private Long contentLength = 0L;
Copy link
Contributor

Choose a reason for hiding this comment

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

im not sure about storing the content length in the state itself. while receiving the entity body, there is a possibility for the state to change right? if it changes, when receiving the response body again, a new ReceivingEntityBody state will be created that doesn't have the previous contentLength value. This is a possibility right? @TharmiganK

Copy link

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

@AzeemMuzammil AzeemMuzammil marked this pull request as ready for review May 28, 2024 05:49
Copy link

sonarcloud bot commented May 28, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants