-
Notifications
You must be signed in to change notification settings - Fork 522
[Azure Logs / Custom Azure Logs] Switch default processor to v2 #16618
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
Conversation
e784951 to
721ea15
Compare
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
84c2dbe to
9220bbb
Compare
🚀 Benchmarks reportPackage
|
| Data stream | Previous EPS | New EPS | Diff (%) | Result |
|---|---|---|---|---|
events |
29411.76 | 17857.14 | -11554.62 (-39.29%) | 💔 |
graphactivitylogs |
2695.42 | 1848.43 | -846.99 (-31.42%) | 💔 |
To see the full report comment with /test benchmark fullreport
| ### Event Hub Processor v2 ✨ | ||
|
|
||
| The integration v2 preview offers a new processor v2 starting with integration version 1.23.0. | ||
| As of version 1.23.0, the integration preview includes the new Processor v2. |
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.
what does "the integration preview" mean?
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.
We introduced the new events data stream as the entry point and router for the Azure Logs integration package.
I plan to make the events data stream (aka Azure Logs v2) as the only data stream running an input.
Unfortunately, years ago we updated the Azure Logs packages to allow multiple inputs to read from the same event hub, but this is a fatal flaw. We need to get rid of this.
The other data streams (activitylogs, firewall_logs, etc) are going to stay without an input.
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.
We marked this events data stream named Azure Logs v2 as "preview" because we were not 100% sure this approach would work. But it seems working pretty well.
| * Features a more efficient checkpoint store leveraging Azure Blob Storage metadata. | ||
|
|
||
| The processor v2 is in preview. Processor v1 is still the default and is recommended for typical use cases. | ||
| Processor v2 is now the default and is recommended for most use cases. Processor v1 remains available as a legacy option for backward compatibility until the next major release. |
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.
most use cases
is there somewhere we can link that details when it's not recommended?
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.
Um, no, there aren't known use cases. Maybe change this to only inform users v1 is still available for some time?
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.
I clarified the description. Let me know if it makes sense.
| (processor v2 only) Flag to control whether the processor should perform the checkpoint information migration from v1 to v2 at startup. The checkpoint migration converts the checkpoint information from the v1 format to the v2 format. | ||
|
|
||
| Default is `false`, which means the processor will not perform the checkpoint migration. | ||
| Default is `true`, which means the processor will attempt to migrate the checkpoint information from v1 to v2 the first time you v1 to v2. |
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.
what happens if you have processor_version: v2 and processor_update_interval: false?
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.
and processor_update_interval: false?
I assume you mean migrate_checkpoint: false.
In that case, the input will start over from the beginning of the stream. If the retention is set to 1 hour, the input will re-process the last hour's worth of events.
I think we should make this explicit in the description rather than leaving it implied, right?
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.
@tommyers-elastic, I’ve fleshed out the migrate_checkpoint description. I think this version is much more comprehensive.
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.
thanks!
| {{/if}} | ||
| {{#if storage_account_key}} | ||
| storage_account_key: {{storage_account_key}} | ||
| storage_account_connection_string: DefaultEndpointsProtocol=https;AccountName={{storage_account}};AccountKey={{storage_account_key}};EndpointSuffix={{endpoint_suffix}} |
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.
nit: i'm guessing storage_account shouldn't be empty if storage_account_key is set, but since it's not in the {{#if storage_account}} there is a chance it's unset.
same with endpoint_suffix
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.
- The
storage_accountcontains the storage account name; this option must be set for the input to work. - The
storage_account_keycontains the auth for processor v1. - The
storage_account_connection_stringcontains the auth for processor v2
We'll have the opportunity to clean things up a little when we'll drop v1 in a couple of iterations.
| # | ||
| # Processor v2 only settings | ||
| # | ||
| - name: processor_version |
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.
are we ok that integration upgrades are going to automatically enable v2 without the user explicitly choosing?
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.
We need to push users to the processor v2 while v1 is still available as fallback option. So if v2 falls short for any reason, users can still fallback to v1 while we address the problem.
We need to drop v11 as soon as possible, since the v1's libraries are so old that sticking with them is becoming a risk.
Footnotes
-
drop v1 as an option and remove it from the Beats codebase. ↩
|
@tommyers-elastic, let me know if there's anything else you'd like me to tweak! |
So v1 checkpoint information will be migrated to v2 on the fist v2 execution.
# Conflicts: # packages/azure/changelog.yml
We retain v1 as legacy option for backwards compatibility only, just in case we run into an unknown issues.
62991aa to
7edce00
Compare
💚 Build Succeeded
History
cc @zmoog |
|
Package azure - 1.34.0 containing this change is available at https://epr.elastic.co/package/azure/1.34.0/ |
|
Package azure_logs - 0.5.0 containing this change is available at https://epr.elastic.co/package/azure_logs/0.5.0/ |
Proposed commit message
Makes the processor v2 the default processor for all integrations in the Azure Logs and Custom Azure Logs packages. The processor v1 is still available as option, but it's listed as "legacy".
The processor v1 uses deprecated libraries and it will be removed in the next release (targeting 9.4).
Checklist
changelog.ymlfile.I have verified that Kibana version constraints are current according to guidelines.I have verified that any added dashboard complies with Kibana's Dashboard good practicesRelated issues