-
Notifications
You must be signed in to change notification settings - Fork 0
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
parsing requestId from message and stamping it as attribute #12
base: main
Are you sure you want to change the base?
Conversation
src/cloudwatch/cloudwatch.go
Outdated
@@ -60,6 +65,12 @@ func batchLogEntries(cloudwatchLogsData events.CloudwatchLogsData, channel chan | |||
Timestamp: strconv.FormatInt(record.Timestamp, 10), | |||
Log: message, | |||
} | |||
if strings.HasPrefix(cloudwatchLogsData.LogGroup, common.LambdaLogGroup) { |
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.
nit : I don't think this if check needs to be a part of the for loop since every message is going to have the same log group
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.
@hrai-nr
I don't think pr actually implements the feature similar to previous lambda implementation.
This is how the previous lambda worked :
Log Line 1 : START RequestId: d653fb2c-0234-46ff-ae6b-9a418b888420 Version: $LATEST INFO ERROR hrai
Log Line 2 : some random log
If this was processed by previous lambda it would append the requestId to both the log lines.
The current implementation would not append the request Id to subsequent requests.
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 would suggest you to move the parsing logic to message_util. it would help in reusability and unit testing.
Also please write unit tests
This PR is for supporting the existing requestId feature from our previous implementation where we were parsing each message and if the message has "RequestId: UUID", we are parsing it and adding it as an attribute.
In our earlier implementation this attribute was named aws.lambda_request_id, but in this implementation we will be using the attribute name as requestId. This is done to make it similar to the key name which aws is sending when the log type is of json.
This change is only applicable to log message which are of type normal text message. For Json type message we are not changing anything.
Testing Done:
Positive case: Send a valid message "START RequestId: d653fb2c-0234-46ff-ae6b-9a418b888420 Version: $LATEST INFO ERROR hrai", the message will flow to new relic and an attribute named requestId with value d653fb2c-0234-46ff-ae6b-9a418b888420 is attached to the logs in new relic.
Positive case: Send a valid message with 2 UUID "START RequestId: d653fb2c-0234-46ff-ae6b-9a418b888420 Version: $LATEST START RequestId: d653fb2c-0234-46ff-ae6b-9a418b888422 Version: $LATEST hrai INFO ERROR", the message will flow to new relic and an attribute named requestId with value d653fb2c-0234-46ff-ae6b-9a418b888420 (first occurence of UUID)is attached to the logs in new relic. (This is just a test case and we won't get two UUID in same message.)
Send a json:
{
"time": "2024-12-12T11:40:11Z",
"level": "INFO",
"message": "hrai hello",
"logger": "root",
"requestId": "6bff3185-1cee-4c13-addb-0c038a197abc"
}
here also the attribute named requestId with value 6bff3185-1cee-4c13-addb-0c038a197abc will be attached to the logs. ( This was working earlier as well, no changes, just testing regression)
Send a complex JSON:
{
"time": "2024-12-12T11:40:11.054Z",
"type": "platform.report ERROR",
"record": {
"requestId": "6bff3185-1cee-4c13-addb-0c038a197abc",
"metrics": {
"durationMs": 9.335,
"billedDurationMs": 10,
"memorySizeMB": 128,
"maxMemoryUsedMB": 30,
"initDurationMs": 92.552
},
"status": "success",
"level": "INFO"
}
}
Here also the attribute named record.requestId with value 6bff3185-1cee-4c13-addb-0c038a197abc will be attached to the logs. ( This was working earlier as well, no changes, just testing regression)
Negative test case: Send messge: "START RequestId:d653fb-0234-46ff-ae6b-9a418b888420 Version: $LATEST INFO ERROR hrai", since it won't match our regex we won't attach any attribute to the logs.
Negative test case: Send messge: "START RequestId: d653fb-0234-46ff-ae6b-9a418b888420 Version: $LATEST INFO ERROR hrai", since it won't match our regex we won't attach any attribute to the logs. (The UUID is not valid UUID)