feat: add endpoint suffix override support for azure#14986
feat: add endpoint suffix override support for azure#14986Kavindu-Dodan merged 7 commits intoelastic:mainfrom
Conversation
1e209d0 to
8cbd589
Compare
🚀 Benchmarks reportTo see the full report comment with |
zmoog
left a comment
There was a problem hiding this comment.
Looks good overall!
We need to make some changes because the "endpoint suffix" only works with the processor v2, and the only integration that allows using the processor v2 is the one named “Collect all Azure Logs (v2 preview)”.
Here's a screenshot with a few comments.
Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> # Conflicts: # packages/azure/changelog.yml # Conflicts: # packages/azure/changelog.yml
Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co>
07bfcaf to
c3cd7d1
Compare
|
@zmoog thanks for the review. I have now focused this change only for Azure V2 stream. Please have a look when you have time. |
zmoog
left a comment
There was a problem hiding this comment.
It's okay to add the parameter to the azure_logs input package because it supports the v2 processor.
There was a problem hiding this comment.
Sorry for not being more precise in the previous comments.
We should add the endpoint_suffix config option to all integrations that support the v2 processor.
For the azure package, it's only needed in packages/azure/data_stream/events. But adding it to azure_logs package as well is a great idea—I hadn't thought of that initially.
Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co>
|
@zmoog thanks for the review 🙏 Agree on the suggestion to add this to |
zmoog
left a comment
There was a problem hiding this comment.
Thanks! Taking a test drive on my local env.
Co-authored-by: Maurizio Branca <maurizio.branca@elastic.co>
Co-authored-by: Maurizio Branca <maurizio.branca@elastic.co>
Thank you and I have enabled auto-merge :) I will loop back if you see the need for more changes. |
zmoog
left a comment
There was a problem hiding this comment.
We only need a small fix to the policy template.
I applied it locally, and here's two snippets of the policy with the default value, and with the custom suffix:
# default: core.windows.net
inputs:
- id: azure-eventhub-events-f55b8918-08b3-4ed4-a710-6a7d4f5e6af6
name: azure-1
revision: 4
type: azure-eventhub
use_output: default
meta:
package:
name: azure
version: 1.28.5
data_stream:
namespace: default
package_policy_id: f55b8918-08b3-4ed4-a710-6a7d4f5e6af6
streams:
- id: azure-eventhub-azure.events-f55b8918-08b3-4ed4-a710-6a7d4f5e6af6
data_stream:
dataset: azure.events
type: logs
elasticsearch:
dynamic_dataset: true
dynamic_namespace: true
connection_string: ${SECRET_0}
storage_account_container: filebeat-events-myeventhub
eventhub: myeventhub
consumer_group: $Default
storage_account: mystorageaccount
storage_account_key: ${SECRET_1}
storage_account_connection_string: >-
DefaultEndpointsProtocol=https;AccountName=mystorageaccount;AccountKey=${SECRET_1};EndpointSuffix=core.windows.net
processor_version: v2
processor_update_interval: 10s
processor_start_position: earliest
partition_receive_timeout: 5s
partition_receive_count: 100
tags:
- azure-eventhub
- forwarded
publisher_pipeline.disable_host: true
sanitize_options: null# custom: core.usgovcloudapi.net
inputs:
- id: azure-eventhub-events-f55b8918-08b3-4ed4-a710-6a7d4f5e6af6
name: azure-1
revision: 5
type: azure-eventhub
use_output: default
meta:
package:
name: azure
version: 1.28.5
data_stream:
namespace: default
package_policy_id: f55b8918-08b3-4ed4-a710-6a7d4f5e6af6
streams:
- id: azure-eventhub-azure.events-f55b8918-08b3-4ed4-a710-6a7d4f5e6af6
data_stream:
dataset: azure.events
type: logs
elasticsearch:
dynamic_dataset: true
dynamic_namespace: true
connection_string: ${SECRET_0}
storage_account_container: filebeat-events-myeventhub
eventhub: myeventhub
consumer_group: $Default
storage_account: mystorageaccount
storage_account_key: ${SECRET_1}
storage_account_connection_string: >-
DefaultEndpointsProtocol=https;AccountName=mystorageaccount;AccountKey=${SECRET_1};EndpointSuffix=core.usgovcloudapi.net
processor_version: v2
processor_update_interval: 10s
processor_start_position: earliest
partition_receive_timeout: 5s
partition_receive_count: 100
tags:
- azure-eventhub
- forwarded
publisher_pipeline.disable_host: true
sanitize_options: nullSigned-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co>
zmoog
left a comment
There was a problem hiding this comment.
Now the policy template for the azure and azure_logs packages render correctly.
| storage_account_key: {{storage_account_key}} | ||
| {{#if endpoint_suffix}} | ||
| storage_account_connection_string: DefaultEndpointsProtocol=https;AccountName={{storage_account}};AccountKey={{storage_account_key}};EndpointSuffix={{endpoint_suffix}} | ||
| {{else}} |
There was a problem hiding this comment.
As the default value is set already for the endpoint_suffix config. Is it okay to remove the else condition? WDYT?
Consider making the endpoint_suffix required:true when removing the else loop.
This is applicable to the azure_logs as well.
There was a problem hiding this comment.
@muthu-mps good point. I added the default value to show what we use internally. So it makes sense to make it mandatory and keep the default in the UI along with simple logic internally.
Added this change with 309ff31 :)
Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co>
💚 Build Succeeded
History
|
|
|
Package azure - 1.28.5 containing this change is available at https://epr.elastic.co/package/azure/1.28.5/ |
|
Package azure_logs - 0.4.1 containing this change is available at https://epr.elastic.co/package/azure_logs/0.4.1/ |
* feat: add endpoint suffix override support for azure Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> # Conflicts: # packages/azure/changelog.yml # Conflicts: # packages/azure/changelog.yml * review changes Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> * review changes Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> * Update packages/azure_logs/manifest.yml Co-authored-by: Maurizio Branca <maurizio.branca@elastic.co> * Update packages/azure/data_stream/events/manifest.yml Co-authored-by: Maurizio Branca <maurizio.branca@elastic.co> --------- Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> Co-authored-by: Maurizio Branca <maurizio.branca@elastic.co>
Proposed commit message
Introduce
endpoint_suffixconfigurations for Azure integrations so that theEndpointSuffixof the Connection String can be overridden based on the deployment environment.Default -
core.windows.netPossible overdding options :
core.usgovcloudapi.netChecklist
changelog.ymlfile.Screenshots