Adding remote resources upgrading mysql #190
Conversation
… when fetching from Logs
There was a problem hiding this comment.
I don't see the change taking effect in the test run you provided: link
If I'm just missing it, could you point it out?
EDIT: Meant to delete this comment before sending out the review comments
| @@ -139,6 +141,10 @@ private Map<String, Object> getActualLog( | |||
| dependencyFilter = String.format("&& ($.RemoteService = \"%s\") && ($.RemoteOperation = \"%s\")", remoteService, remoteOperation); | |||
| } | |||
|
|
|||
| if (remoteResourceType != null && remoteResourceIdentifier != null) { | |||
| dependencyFilter += String.format(" && ($.RemoteResourceType = %%%s%%) && ($.RemoteResourceIdentifier = %%%s%%)", remoteResourceType, remoteResourceIdentifier); | |||
There was a problem hiding this comment.
If we expect to be searching with the value directly, we can likely just remove the ^...$ RegEx symbols in the validation templates and remove the need for RegEx here altogether. Not a 100% necessary change but doing so will improve the search time by filtering for the exact string instead of a RegEx pattern.
There was a problem hiding this comment.
Hey Mahad, we shouldn't expect the same value for either remoteResourceType or remoteResourceIdentifier. You can verify this in the following run: GitHub Workflow Run.
- The /aws-sdk-call API shows different values.
- The /mysql API also has different values.
I believe these values can't be static since some of them are concatenated into a string only during runtime. Let me know if I've missed anything.
There was a problem hiding this comment.
Hi @ektabj
I think Mahad suggests to not use regex here.
As you can see, we are not using regex for RemoteResourceType in the template.
For RemoteResourceIdentifier, we can change the value from ^{{remoteResourceIdentifier}}$ to {{remoteResourceIdentifier}} in the template.
Sorry, maybe i'm missing something on my end, but I think I can see the change in effect for example these two log lines: I see link is tagged with a line with log "Filter Pattern for Log Search" which wasn't changed in this PR. |
Original PR: #103
Copying description here.
Issue description:
When we assert
RemoteResourceTypeandRemoteResourceIdentifierattributes in EMF logs, we are using Logs filter:RemoteServiceandRemoteOperationfilter to fetch logs. However, it's not guaranteed that EMF log withRemoteServiceandRemoteOperationattributes will haveRemoteResourceTypeandRemoteResourceIdentifier. The logs fetched with this generic filter might get more logs than expected and only asserting the first log event in the result.Description of changes:
This change enhances the log filter by appending
&& ($.RemoteResourceType = %EXAMPLE%) && ($.RemoteResourceIdentifier = %EXAMPLE%)to the filter if they present in the template to get more specific log events.Log Filter doc: https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/FilterAndPatternSyntax.html#regex-expressions
Log Filter used before this change (From E2E testing log):
Log Filter used after this change (From E2E testing log):
e2e test run: https://github.com/ektabj/aws-application-signals-test-framework/actions/runs/10580614526/job/29315930502 on
us-east-1thanks to Ekta.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.