[Azure] [PlatformLogs] Fix pipeline for edge cases #12735
Conversation
lucian-ioan
added
Integration:azure
🚀 Benchmarks reportPackage
|
| Data stream | Previous EPS | New EPS | Diff (%) | Result |
|---|---|---|---|---|
signinlogs |
2551.02 | 1805.05 | -745.97 (-29.24%) | 💔 |
springcloudlogs |
5434.78 | 3984.06 | -1450.72 (-26.69%) | 💔 |
platformlogs |
5747.13 | 4830.92 | -916.21 (-15.94%) | 💔 |
provisioning |
6849.32 | 2801.12 | -4048.2 (-59.1%) | 💔 |
To see the full report comment with /test benchmark fullreport
|
@muthu-mps : Can you please review this. |
| @@ -63,8 +59,7 @@ processors: | |||
| field: azure.resource_id | |||
| if: 'ctx.azure?.subscription_id == null' | |||
| patterns: | |||
| - /SUBSCRIPTIONS/%{SUBID:azure.subscription_id}/RESOURCEGROUPS/%{GROUPID:azure.resource.group} | |||
| - /subscriptions/%{SUBID:azure.subscription_id}/resourceGroups/%{GROUPID:azure.resource.group} | |||
| - /(?i)subscriptionssubscriptions/%{SUBID:azure.subscription_id}/resourceGroups/%{GROUPID:azure.resource.group} | |||
There was a problem hiding this comment.
subscriptionssubscriptions doesn't look right (it's word subscriptions twice`). Is it a mistake?
| @@ -148,7 +148,7 @@ processors: | |||
| - convert: | |||
| field: azure.platformlogs.durationMs | |||
| target_field: event.duration | |||
| type: integer | |||
| type: float | |||
There was a problem hiding this comment.
- Not sure why the
event.durationis of type integer. As we reference the ECS field for event.duration changing the type to float may not align with the ECS field as the event.duration is of type long. You have to convert it to long instead of float. event.duration ECS reference- https://www.elastic.co/guide/en/ecs/current/ecs-event.html#field-event-duration- Will this change create a mapping conflict? If Yes, then lets file a separate issue for this fix.
There was a problem hiding this comment.
event.duration gets converted from azure.platformlogs.durationMs which can be string represeting a float.
Edit: I can convert the original string to long but it can come as "2.7897" so I would lose the precision. WDYT @muthu-mps ?
There was a problem hiding this comment.
Yes, This value is again converted to nano seconds here
So we can keep the type as long.
There was a problem hiding this comment.
@ishleenk17 - Do you have any second thought on this?
There was a problem hiding this comment.
Why do we use event.duration ?
I expect conflicts to happen in case this field is already present and datatype is long.
Can't we use a time duration field specific for azure platform logs ?
There was a problem hiding this comment.
Generally, event.duration is used in all Azure integrations to capture the request duration. I don't think we can add a separate field for platform logs alone.
To fix this we need to change integer field type to long.
There was a problem hiding this comment.
Thank you, I think it will work. Testing this live before merging.
There was a problem hiding this comment.
@muthu-mps : We have never confronted conflicts for this field ?
Also, I assume all the event.duration are long in existing azure logs as well to adhere to the ECS fields.
There was a problem hiding this comment.
We have never confronted conflicts for this field ?
No, We have not seen any conflict error.
Also, I assume all the event.duration are long in existing azure logs as well to adhere to the ECS fields.
yes, all other Azure integrations adhere to ECS field.
There was a problem hiding this comment.
LGTM!
As the field type change doesn't induce any error. cc: @lucian-ioan
💚 Build Succeeded
History
cc @lucian-ioan |
|
|
Package azure - 1.22.2 containing this change is available at https://epr.elastic.co/package/azure/1.22.2/ |
Proposed commit message
Fix pipeline for edge cases.
Checklist
changelog.ymlfile.Author's Checklist
How to test this PR locally
Related issues
Screenshots