M365 Crawler Metric, Buffer, and Unit Test Updates #6142
Conversation
chrisale000
requested review from
KarstenSchnitter,
chenqi0805,
dinujoh,
dlvenable,
engechas,
graytaylor0,
kkondaka,
oeyh,
san81,
sb2k16 and
srikanthjg
as code owners
| new ParameterizedTypeReference<>() {} | ||
| ); | ||
| // Record search request size. | ||
| searchRequestSizeSummary.record(response.getHeaders().getContentLength()); |
There was a problem hiding this comment.
Why are we recording the response header length for the search request size?
There was a problem hiding this comment.
This method returns the size of the body in terms of bytes: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/http/HttpHeaders.html#getContentLength()
I could match the implementation with getAuditLog or vice versa. The main difference here was one is the size of bytes from the string and the other is from responseEntity itself.
chrisale000
force-pushed
the
load-test-ais
branch
from
5295512 to
c808208
Compare
| private static final String AUDIT_LOG_FETCH_LATENCY = "auditLogFetchLatency"; | ||
| private static final String SEARCH_CALL_LATENCY = "searchCallLatency"; | ||
| private static final String AUDIT_LOGS_REQUESTED = "auditLogsRequested"; | ||
| private static final String AUDIT_LOG_REQUEST_SIZE = "auditLogRequestSizeBytes"; |
There was a problem hiding this comment.
Isn't naming these RequestSize a little misleading? It looks like this is measuring the response size / the size of the audit logs?
| if (configuration.isAcknowledgments()) { | ||
| acknowledgementSet.add(record.getData()); | ||
| buffer.write(record, (int) Duration.ofSeconds(BUFFER_TIMEOUT_IN_SECONDS).toMillis()); | ||
| acknowledgementSet.complete(); |
There was a problem hiding this comment.
I do not think this is correct. The "acknowledgementSet.complete();" should be done outside of the loop.
There was a problem hiding this comment.
Or create an acknowledgementset inside the loop
There was a problem hiding this comment.
Thank you for the feedback. You are correct - I will move the acknowledgementSet.Add(record.getData()) outside of the loop to ensure proper handling of the acknowledgment set. This will prevent potential duplicate events in the acknowledgementSet.
There was a problem hiding this comment.
Also, pushed acknowledgementSet.complete() to execute partition logic where all events are being processed.
chrisale000
force-pushed
the
load-test-ais
branch
from
f740a1b to
ddc117c
Compare
chrisale000
force-pushed
the
load-test-ais
branch
from
ddc117c to
ad597c2
Compare
4 participants
Description
This pull request implements comprehensive monitoring enhancements and performance optimizations:
Core Functionality:
Testing and Validation:
The changes have been thoroughly tested and documented, with all tests passing successfully. The implementation follows established coding standards and includes appropriate unit test coverage.
Issues Resolved
N/A
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.