-
Notifications
You must be signed in to change notification settings - Fork 511
[Azure App Service] bug fixes #15591
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
[Azure App Service] bug fixes #15591
Conversation
…tions # Conflicts: # packages/statsd_input/changelog.yml # packages/statsd_input/manifest.yml
This reverts commit f9c172f.
🚀 Benchmarks reportPackage
|
| Data stream | Previous EPS | New EPS | Diff (%) | Result |
|---|---|---|---|---|
app_service_logs |
4608.29 | 1669.45 | -2938.84 (-63.77%) | 💔 |
To see the full report comment with /test benchmark fullreport
| - set: | ||
| field: azure.app_service.properties_raw | ||
| value: "{{{azure.app_service.properties}}}" |
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.
Why is this renaming needed?
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.
Because in appservice-platformlogs-inner-pipeline.yml in azure.app_service.log field we were setting value of azure.app_service.properties which were expected as string (actually it's json string).
But in other pipelines we need to decode that JSON in order to get values of decoded fields.
So this properties_raw is just temporary field in order to set azure.app_service.log field in appservice-platformlogs-inner-pipeline.yml
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.
Will test this PR when it's ready for review. It also needs a changelog entry.
@giorgi-imerlishvili-elastic - Can you address this comment? |
I was thinking to make it ready for review once this is merged #15487 |
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.
Pipeline improvements LGTM.
| ignore_failure: true | ||
| - set: | ||
| field: azure.app_service.properties_raw | ||
| value: "{{{azure.app_service.properties}}}" |
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.
@giorgi-imerlishvili-elastic - Can we add comment about this change. Otherwise this seems to be like adding a field and removing the same field without any processing. Also in the platformlogs-inner-pipeline the properties field is removed. Is it valid?
# Conflicts: # packages/azure_app_service/changelog.yml # packages/azure_app_service/manifest.yml
💚 Build Succeeded
History
|
|
Package azure_app_service - 0.10.0 containing this change is available at https://epr.elastic.co/package/azure_app_service/0.10.0/ |
Proposed commit message
Fix: [Azure App Service]: Expects JSON object but is unparsed JSON string #11169
Fix: [Azure App Service] Missing azure.subscription_id #11172
Checklist
changelog.ymlfile.Author's Checklist
How to test this PR locally
Related issues
Screenshots