-
Notifications
You must be signed in to change notification settings - Fork 474
Add var.storage_account_container in Azure Logs #3877
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
Add var.storage_account_container in Azure Logs #3877
Conversation
We need to make some changes: defining the Each azureeventhub input uses a storage account container to store checkpoint information about its consumer group, for example: {
"partitionID": "0",
"epoch": 1,
"owner": "b5454f29-881f-48ae-bd34-5711932f378e",
"checkpoint": {
"offset": "47244772552",
"sequenceNumber": 986,
"enqueueTime": "2022-07-20T08:17:37.523Z"
},
"state": "available",
"token": "923168d8-bc7a-4e75-8f70-4afb83637c12"
} |
good catch! thanks for taking care of this |
I am positioning it in the "advanced" section because it requires a deeper knowledge to set it properly and avoid side effects.
🌐 Coverage report
|
From the technical perspective, to keep things working as expected, we must place the From the UI perspective, we have the choice: can make it always visible and put it in the "standard" section (2) or put it in the "advanced" section (3) of the data stream. Since this is a vital option and can mess things up if used by multiple integrations, I am inclined to pick the "advanced" option. I'd love to hear what you think. cc @ravikesarwani @kaiyan-sheng The description is also a critical aspect of this change. I started with this description (this is a draft, do not consider it final):
The goal is to convey the following information:
|
@zmoog I also vote for putting it under the advanced options. But instead of having it all the way to the bottom of the list, I would put it before tags and processors since tags and processors are common across all data streams basically. btw this can be considered separately. We can also add the Edit: I see some fields are already specified as optional for azure. I guess we are missing the |
Yes, let's put it in the Advanced option please since this will be used only in some use cases. |
Sounds good; let's go for the "advanced" option. I expect this option to be used in rare cases, but it needs to be there for a few. Positioning it before the tags looks like a better alternative; thanks @kaiyan-sheng! I will double-check the use of the |
I also moved other data stream options to match the order they appear on the settings page.
Updated the PR description with the a screenshot of the up-to-date version of integration settings. |
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.
nice work - the description of the new field is super clear.
Enhancement
What does this PR do?
Add var.storage_account_container in Azure Logs
Checklist
changelog.yml
file.Author's Checklist
Related issues
Screenshots
The new "Storage Account Container" option is positioned in the advanced section of each data stream: