Upgrade azure-eventhub to the new Event Hub SDK #39796

zmoog · 2024-06-04T16:06:59Z

Proposed commit message

Restructure the azure-eventhub input, rebranding the current version as processor v1. Add a brand new processor v2, allowing users to select which version to use in the config:

processor v1: uses the legacy Event Hub SDK (default processor, at least for 8.15)
processor v2: uses the modern Event Hub SDK

Why are we introducing a processor v2?

processor v1 uses deprecated libraries
- github.com/Azure/azure-event-hubs-go (legacy)
- github.com/Azure/azure-storage-blob-go (legacy, retiring on Sep 2024)
processor v1 does not support publishing acks (mostly due to lack of hooks; the legacy SDK is a black box)

Notes for reviewers

Overview

To help with the review, here is an overview of the main flow of the processor v2-based input.

The processor v2 starts a new consumer for each event hub partition.
Each consumer creates a pipeline client.
When a consumer receives an event, it decodes it and sends it to the pipeline client.
When the pipeline successfully processes the event, it acknowledges with the consumer.
The consumer stores the sequence number of the last successful event in the partition blob in the storage account container.

New features

Replace the legacy SDK with the new modern and supported SDK
Add support for publishing ACKs
Add a migration assistant to migrate checkpoint v1 information to the v2 format

Replace the legacy SDK with the new modern and supported SDK

The new SDK is more flexible and allows us to implement new features and configuration options.

Add support for publishing ACKs

Now, the processor v2 updates the sequence number only when the events have been successfully delivered to Elasticsearch.

Add a migration assistant to migrate checkpoint v1 information to the v2 format

On the first start of the processor v2, the migration assistant (enabled by default) checks if checkpoint v1 information exists from processor v1 and migrates them to the v2 format.

See "Scenario 001: Migration" at x-pack/filebeat/input/azureeventhub/README.md for more details.

New configuration options

There are new configuration options for v2:

storage_account_connection_string (required) to authenticate with the storage account container.
migrate_checkpoint (optional, default: yes) controls if the processor v2 should check and migrate checkpoint v1 information on start.
processor_version (optional, default: v1) which processor version to use.
processor_update_interval (optional, default: 10s) time interval between checking if new partitions are available.
processor_start_position (optional, default: earliest) controls if the processor should start from the beginning earliest or the latest event in the event hub retention period.
partition_receive_timeout (optional, default: 5s)
partition_receive_count (optional, default: 100)

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Disruptive User Impact

How to test this PR locally

See "Test Scenarios" section in the x-pack/filebeat/input/azureeventhub/README.md file.

Related issues

Closes [Azure] Migrate the azure-eventhub input to the new Azure Event Hubs Client Module for Go #33815

Use cases

Screenshots

Logs

Author's Checklist

Give feedback

Test that the migrationAssistant returns a "failed to download checkpoint v1 information for partition"
Test that the migrationAssistant returns a "failed to parse offset"
Compare Storage Account metrics (transactions, blob capacity, blob count, egress, ingress) between v1 and v2
Performance testing with 1K, 10K, 100K, 1M log events
Options

mergify · 2024-06-04T16:07:41Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @zmoog? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

elasticmachine · 2024-06-05T07:11:50Z

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

elasticmachine · 2024-06-05T07:12:24Z

Pinging @elastic/obs-ds-hosted-services (Team:obs-ds-hosted-services)

botelastic · 2024-06-05T07:12:28Z

This pull request doesn't have a Team:<team> label.

gizas · 2024-07-05T12:36:27Z

x-pack/filebeat/input/azureeventhub/input_test.go

@@ -93,58 +99,6 @@ func TestProcessEvents(t *testing.T) {
 	assert.Equal(t, message, single)
 }

-func TestParseMultipleRecords(t *testing.T) {


Why you removed those? Because you think that v1 is not going to be used anymore?
And you have the decoder_test for this?

I moved the message parsing to the decoder to share it with the v1 and v2 processors. The decoder has its own tests based on the original v1 processor tests.

mergify · 2024-07-05T12:37:05Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b zmoog/azure-eventhub-sdk-upgrade upstream/zmoog/azure-eventhub-sdk-upgrade
git merge upstream/main
git push upstream zmoog/azure-eventhub-sdk-upgrade

gizas · 2024-07-05T12:39:13Z

x-pack/filebeat/input/azureeventhub/v2_migration.go

+	consumerGroup string) error {
+
+	// v2 checkpoint information path
+	// mbranca-general.servicebus.windows.net/sdh4552/$Default/checkpoint/0


Delete this line?

Uhm, I need to rephrase this comment to make it more useful.

x-pack/filebeat/input/azureeventhub/v2_migration.go

gizas · 2024-07-05T12:42:18Z

x-pack/filebeat/input/azureeventhub/v2_migration.go

+
+	size, err := cln.DownloadBuffer(ctx, buff[:], nil)
+	if err != nil {
+		return fmt.Errorf("failed to download checkpoint v1 information for partition %s: %w", partitionID, err)


Will this error be printed? I think you have to introduce a log.Error here

If the migrationAssistant fails to migrate a partition, the run() function should print all the wrapped errors.

I will add a test to double-check it happens.

gizas · 2024-07-05T12:42:42Z

x-pack/filebeat/input/azureeventhub/v2_migration.go

+
+	offset, err := strconv.ParseInt(checkpointV1.Checkpoint.Offset, 10, 64)
+	if err != nil {
+		return fmt.Errorf("failed to parse offset: %w", err)


Same as above

I'll test this as well.

Alongside the partition ID, users can optional send event with a partition key. Add an (optional) partition key to the event hub metadata.

The new migrate_checkpoint config option controls if the input v2 should perform a migration check on start. If migrate_checkpoint is true, the input checks and performs the migration (if v1 info exists) on the very first v2 run. If migrate_checkpoint is false, the input will skip the migration assistant and will not perform any checks or migration.

Expand processor options by adding a new `start_position` configuration. Possible values for `start_position` are: - "earliest" to start from the beginning of the event hub retention period. - "latest" to start from new events. The input uses the 'start_position' option when checkpoint information from the storage account container is unavailable (on the input's first start).

Makes the receive configuration settings available for customization on the input settings. The current default values (receive_timeout: 5s, receive_count: 100) are probably fine, but it is better to make these options available to users.

Co-authored-by: Andrew Gizas <[email protected]> Co-authored-by: subham sarkar <[email protected]>

Also update the option description as well adding the default value.

It's better to check the private data in the event has the expected type.

Adding more details to the message logged on successful store: - sequence_number - offset - enqueued_time

The teardown() function is responsible to release all the resources allocated in the setup() function.

shmsr

Echoing Fae's comment:

Great work and I'm happy to see this quality of code going in :-)

I agree. The code is well-structured and idiomatically written, accompanied by clear and informative comments. LGTM — approving!

Restructure the `azure-eventhub` input, rebranding the current version as processor v1. Add a brand new processor v2, allowing users to select which version to use in the config: - processor v1: uses the [legacy](https://github.com/azure/azure-event-hubs-go) Event Hub SDK (default processor, at least for 8.15) - processor v2: uses the [modern](https://github.com/azure/azure-sdk-for-go/blob/main/sdk/messaging/azeventhubs/) Event Hub SDK Why are we introducing a processor v2? - processor v1 uses deprecated libraries - [github.com/Azure/azure-event-hubs-go](http://github.com/Azure/azure-event-hubs-go) (legacy) - [github.com/Azure/azure-storage-blob-go](http://github.com/Azure/azure-storage-blob-go) (legacy, [retiring](https://azure.microsoft.com/en-gb/updates/retirement-notice-the-legacy-azure-storage-go-client-libraries-will-be-retired-on-13-september-2024/) on Sep 2024) - processor v1 does not support publishing acks (mostly due to lack of hooks; the legacy SDK is a black box) --------- Co-authored-by: Tiago Queiroz <[email protected]> Co-authored-by: Andrew Gizas <[email protected]> Co-authored-by: subham sarkar <[email protected]> (cherry picked from commit b95a8a0) # Conflicts: # go.mod # go.sum

…DK (#40455) * Upgrade azure-eventhub to the new Event Hub SDK (#39796) Restructure the `azure-eventhub` input, rebranding the current version as processor v1. Add a brand new processor v2, allowing users to select which version to use in the config: - processor v1: uses the [legacy](https://github.com/azure/azure-event-hubs-go) Event Hub SDK (default processor, at least for 8.15) - processor v2: uses the [modern](https://github.com/azure/azure-sdk-for-go/blob/main/sdk/messaging/azeventhubs/) Event Hub SDK Why are we introducing a processor v2? - processor v1 uses deprecated libraries - [github.com/Azure/azure-event-hubs-go](http://github.com/Azure/azure-event-hubs-go) (legacy) - [github.com/Azure/azure-storage-blob-go](http://github.com/Azure/azure-storage-blob-go) (legacy, [retiring](https://azure.microsoft.com/en-gb/updates/retirement-notice-the-legacy-azure-storage-go-client-libraries-will-be-retired-on-13-september-2024/) on Sep 2024) - processor v1 does not support publishing acks (mostly due to lack of hooks; the legacy SDK is a black box) --------- Co-authored-by: Tiago Queiroz <[email protected]> Co-authored-by: Andrew Gizas <[email protected]> Co-authored-by: subham sarkar <[email protected]>

zmoog self-assigned this Jun 4, 2024

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jun 4, 2024

This was referenced Jun 4, 2024

Upgrade azure-eventhub input to the modern Event Hub SDK zmoog/beats#1

Closed

[Azure] Migrate the azure-eventhub input to the new Azure Event Hubs Client Module for Go #33815

Closed

zmoog marked this pull request as ready for review June 4, 2024 21:55

zmoog requested review from a team as code owners June 4, 2024 21:55

zmoog requested review from faec, leehinman and a team June 4, 2024 21:55

pierrehilbert added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Jun 5, 2024

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jun 5, 2024

zmoog force-pushed the zmoog/azure-eventhub-sdk-upgrade branch from 88abdd1 to 9afb38a Compare June 28, 2024 06:06

gizas reviewed Jul 5, 2024

View reviewed changes

x-pack/filebeat/input/azureeventhub/v2_migration.go Outdated Show resolved Hide resolved

gizas reviewed Jul 5, 2024

View reviewed changes

zmoog and others added 16 commits August 2, 2024 12:40

Add partition ID to event hub metadata

eedded6

minor: comments

a871280

Add support for partition key in eventhub metadata

fedfcf4

Alongside the partition ID, users can optional send event with a partition key. Add an (optional) partition key to the event hub metadata.

Add README with detailed test scenarios

d539017

Add Scenario 004: Invalid Elasticsearch endpoint

d0a37b0

READNE cleanup

0b7fd84

Add receive_timeout and receive_count to config

2c6044f

Makes the receive configuration settings available for customization on the input settings. The current default values (receive_timeout: 5s, receive_count: 100) are probably fine, but it is better to make these options available to users.

Update tests

e2f0a43

Fix linter complaints and cleanups

769d199

make update

2eaa9ac

Apply suggestions from code review

cb62264

Co-authored-by: Andrew Gizas <[email protected]> Co-authored-by: subham sarkar <[email protected]>

Clarify comment on v2 checkpoint info path

91c0229

Add config validation on new options

0f33396

Also update the option description as well adding the default value.

Double-check private data in acknowledged events

feb52b7

It's better to check the private data in the event has the expected type.

zmoog force-pushed the zmoog/azure-eventhub-sdk-upgrade branch from 52b3fb8 to feb52b7 Compare August 2, 2024 10:41

zmoog added 2 commits August 2, 2024 12:51

Add sequence number and offset to checkpoint log

9361b56

Adding more details to the message logged on successful store: - sequence_number - offset - enqueued_time

Add teardown() function on the v2 input

53925c0

The teardown() function is responsible to release all the resources allocated in the setup() function.

zmoog requested a review from shmsr August 2, 2024 11:13

Update NOTICE.txt

03f0625

shmsr approved these changes Aug 5, 2024

View reviewed changes

leehinman approved these changes Aug 5, 2024

View reviewed changes

Pick the right value from range loop

fa5f346

zmoog merged commit b95a8a0 into elastic:main Aug 7, 2024
122 checks passed

zmoog deleted the zmoog/azure-eventhub-sdk-upgrade branch August 7, 2024 23:47

zmoog added the backport-8.15 Automated backport to the 8.15 branch with mergify label Aug 8, 2024

zmoog added the input:azure-eventhub label Aug 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade azure-eventhub to the new Event Hub SDK #39796

Upgrade azure-eventhub to the new Event Hub SDK #39796

zmoog commented Jun 4, 2024 •

edited

Loading

Author's Checklist

mergify bot commented Jun 4, 2024

elasticmachine commented Jun 5, 2024

elasticmachine commented Jun 5, 2024

botelastic bot commented Jun 5, 2024

gizas Jul 5, 2024 •

edited

Loading

zmoog Jul 29, 2024

mergify bot commented Jul 5, 2024

gizas Jul 5, 2024

zmoog Jul 29, 2024

gizas Jul 5, 2024 •

edited

Loading

zmoog Jul 29, 2024

gizas Jul 5, 2024

zmoog Jul 29, 2024

shmsr left a comment

Upgrade azure-eventhub to the new Event Hub SDK #39796

Upgrade azure-eventhub to the new Event Hub SDK #39796

Conversation

zmoog commented Jun 4, 2024 • edited Loading

Proposed commit message

Notes for reviewers

Overview

New features

Replace the legacy SDK with the new modern and supported SDK

Add support for publishing ACKs

Add a migration assistant to migrate checkpoint v1 information to the v2 format

New configuration options

Checklist

Disruptive User Impact

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

Author's Checklist

mergify bot commented Jun 4, 2024

elasticmachine commented Jun 5, 2024

elasticmachine commented Jun 5, 2024

botelastic bot commented Jun 5, 2024

gizas Jul 5, 2024 • edited Loading

Choose a reason for hiding this comment

zmoog Jul 29, 2024

Choose a reason for hiding this comment

mergify bot commented Jul 5, 2024

gizas Jul 5, 2024

Choose a reason for hiding this comment

zmoog Jul 29, 2024

Choose a reason for hiding this comment

gizas Jul 5, 2024 • edited Loading

Choose a reason for hiding this comment

zmoog Jul 29, 2024

Choose a reason for hiding this comment

gizas Jul 5, 2024

Choose a reason for hiding this comment

zmoog Jul 29, 2024

Choose a reason for hiding this comment

shmsr left a comment

Choose a reason for hiding this comment

zmoog commented Jun 4, 2024 •

edited

Loading

gizas Jul 5, 2024 •

edited

Loading

gizas Jul 5, 2024 •

edited

Loading