Upgrade azure-eventhub to input v2 API #39511

zmoog · 2024-05-10T10:09:36Z

Proposed commit message

Upgrade azure-eventhub to input v2 API.

The input v2 API provides better health status reporting and control. This is a preliminary step in introducing support for the modern Azure Event Hub SDK for Go.

Notes for reviewers

This PR aims to update the azure-eventhub input to the input v2 API without introducing behavior change. The PR is a stepping stone to #33815 (the PR is coming)

Here is a list of changes to keep in mind during the review.

The PR:

Renames the azureInput to eventHubInputV1 because I want to continue offering the legacy SDK while we introduce an eventHubInputV2 with the modern event hub SDK in the next PR. The eventHubInputManager will select the input version depending on the configuration.
Switches the pipeline publishing process from Outlet (input v1) to PipelineClient (input v2).
Starts the EPH and stops it when the input context is "done".
Rename parseMultipleMessages() to unpackRecords() because seems to better represent the behavior.

Keep existing behavior: Beats pipeline acks

It's important to note that the original input version did not handle publishing ACKs to the Beats pipeline. This was primarily because (AFAIK) the input API v1 did not provide this functionality.

With the transition to the input API v2, we have acks management. However, the legacy event hub SDK internally updates the checkpoint info after a successful handler call and does not seem to offer hooks to plug proper ACK management.

Since the new modern SDK offers better checkpoint management, in this PR, we keep the current behavior intact, and we'll implement ACKs in the event hub input v2.

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

Author's Checklist

Test reload configuration

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

mergify · 2024-05-10T10:10:16Z

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-05-22T08:29:34Z

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

belimawr

I took a quick look at the InputV2 API part of the PR and it looks good. I just have a question about an empty file.

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

mergify · 2024-05-24T10:10:04Z

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-to-input-api-v2 upstream/zmoog/azure-eventhub-to-input-api-v2
git merge upstream/main
git push upstream zmoog/azure-eventhub-to-input-api-v2

constanca-m · 2024-06-06T13:11:27Z

I cannot comment the lines you did not change, but is there reason to leave this?

https://github.com/zmoog/beats/blob/cc1d24512dad1a0d8f09055c12cb6e4d49c18ced/x-pack/filebeat/input/azureeventhub/metrics.go#L34

And again in the test:

https://github.com/zmoog/beats/blob/cc1d24512dad1a0d8f09055c12cb6e4d49c18ced/x-pack/filebeat/input/azureeventhub/metrics_test.go#L146

Is it even possible this condition to be true at all?

constanca-m · 2024-06-06T13:17:47Z

x-pack/filebeat/input/default-inputs/inputs_other.go

@@ -31,6 +32,7 @@ import (
 func xpackInputs(info beat.Info, log *logp.Logger, store beater.StateStore) []v2.Plugin {
 	return []v2.Plugin{
 		azureblobstorage.Plugin(log, store),
+		azureeventhub.Plugin(log),


I am a bit confused, what's this for?

This registers azureeventhub (using the input plugin v2 API) and makes it available to Filebeat. Previous, the registration was happening using the old API:

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

Lines 35 to 40 in db9406b

func init() {

err := input.Register(inputName, NewInput)

if err != nil {

panic(fmt.Errorf("failed to register %v input: %w", inputName, err))

}

}

Without registration, the plugin is unknown to Filebeat.

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

mergify · 2024-06-10T15:29:50Z

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-to-input-api-v2 upstream/zmoog/azure-eventhub-to-input-api-v2
git merge upstream/main
git push upstream zmoog/azure-eventhub-to-input-api-v2

zmoog · 2024-06-10T15:37:41Z

I cannot comment the lines you did not change, but is there reason to leave this?

https://github.com/zmoog/beats/blob/cc1d24512dad1a0d8f09055c12cb6e4d49c18ced/x-pack/filebeat/input/azureeventhub/metrics.go#L34

And again in the test:

https://github.com/zmoog/beats/blob/cc1d24512dad1a0d8f09055c12cb6e4d49c18ced/x-pack/filebeat/input/azureeventhub/metrics_test.go#L146

Is it even possible this condition to be true at all?

Your question makes me think I don't fully understand how metrics.NewUniformSample(1024) works! 😄

Can you elaborate on this? Thanks!

elasticmachine · 2024-06-11T09:30:29Z

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

constanca-m · 2024-06-11T10:48:41Z

Your question makes me think I don't fully understand how metrics.NewUniformSample(1024) works! 😄

Indeed, I went to have a look and I think I was confused when I wrote that comment.

I was reading the unit tests to understand the code at first and the TODO there got me a bit confused. From input.processEvents(&ev) I don't know if the condition can ever be untrue, but this is minor anyway.

cmacknz

Approving the go.mod changes

Switch from the v1 Outlet to the v2 PipelineClient

It seems we only need to wait for the context to be done here.

The existing input version does not handle publishing acks from the Beats pipeline. The input API v1 does not seem to offer this feature. With the transition to the input API v2, we have acks management. However, the legacy event hub SDK internally updates the checkpoint info after a successful handler call, and does not seem to offer hooks for acks management. Since the new modern SDK offers better checkpoint management, we keep the current behavior intact, and we'll implement ACKs in the event hub input v2.

Co-authored-by: Tiago Queiroz <me@tiago.life>

Registers the `azure-eventhub` input on the Windows platform. During the recent [upgrade](#39511) of the input from the Filebeat input API v1 to input API v2, I missed registering the input for the Windows platform in the `x-pack/filebeat/input/default-inputs/inputs_windows.go` file.

Registers the `azure-eventhub` input on the Windows platform. During the recent [upgrade](#39511) of the input from the Filebeat input API v1 to input API v2, I missed registering the input for the Windows platform in the `x-pack/filebeat/input/default-inputs/inputs_windows.go` file. (cherry picked from commit cc561ff)

Registers the `azure-eventhub` input on the Windows platform. During the recent [upgrade](#39511) of the input from the Filebeat input API v1 to input API v2, I missed registering the input for the Windows platform in the `x-pack/filebeat/input/default-inputs/inputs_windows.go` file. (cherry picked from commit cc561ff) Co-authored-by: Maurizio Branca <maurizio.branca@elastic.co>

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

mergify bot assigned zmoog May 10, 2024

zmoog changed the title ~~Upgrade azure-eventhub to input v2 API~~ [WIP] Upgrade azure-eventhub to input v2 API May 10, 2024

zmoog added the Team:obs-ds-hosted-services Label for the Observability Hosted Services team label May 10, 2024

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

zmoog force-pushed the zmoog/azure-eventhub-to-input-api-v2 branch 2 times, most recently from 025e137 to 62b90b3 Compare May 20, 2024 08:39

zmoog changed the title ~~[WIP] Upgrade azure-eventhub to input v2 API~~ Upgrade azure-eventhub to input v2 API May 20, 2024

zmoog mentioned this pull request May 21, 2024

[Azure] Upgrade azure-eventhub to Input API v2 #36377

Closed

zmoog marked this pull request as ready for review May 22, 2024 08:29

zmoog requested review from a team as code owners May 22, 2024 08:29

zmoog requested review from belimawr and leehinman May 22, 2024 08:29

zmoog requested a review from faec May 22, 2024 09:39

belimawr reviewed May 22, 2024

View reviewed changes

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

zmoog mentioned this pull request May 23, 2024

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

Closed

zmoog requested a review from a team May 24, 2024 10:09

constanca-m reviewed Jun 6, 2024

View reviewed changes

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

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

cmacknz approved these changes Jun 12, 2024

View reviewed changes

zmoog and others added 19 commits June 27, 2024 07:57

Initial POC import

84a8d3f

Rename client to pipelineClient for clarity

94040f1

Rename azure to eventHubMetadata for clarity

fd4ca94

Move remaining code out of eph.go

0f85bc7

Remove erroneous error check

9fbceab

Update tests

df63134

Switch from the v1 Outlet to the v2 PipelineClient

Address linter complaints

110592c

Simplify run()

3fbe6f2

It seems we only need to wait for the context to be done here.

Update NOTICE

2dbbcc6

Remove the eventHubInputV2 POC

3fd031a

Cleanup

7a3a778

Cleanup

d9f7f3c

Cleanup and add comments

93c2f91

Update tests

91e7b70

Cleanup

b5ed3a5

Cleanup

5cfd5bb

Update x-pack/filebeat/input/azureeventhub/v1_input_test.go

158fc0b

Co-authored-by: Tiago Queiroz <me@tiago.life>

Remove a leftover empty file

b0b04ee

zmoog force-pushed the zmoog/azure-eventhub-to-input-api-v2 branch from cc1d245 to b0b04ee Compare June 27, 2024 15:07

zmoog merged commit 68e2b8b into elastic:main Jun 28, 2024
121 checks passed

zmoog deleted the zmoog/azure-eventhub-to-input-api-v2 branch June 28, 2024 05:12

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

This was referenced Aug 24, 2024

[Azure] azure-eventhub input fails to start on Windows due to "Error creating runner from config: Error creating input. No such input type exist: 'azure-eventhub'" error #40608

Closed

Register the azure-eventhub input on Windows #40609

Merged

mergify bot mentioned this pull request Aug 26, 2024

[8.15](backport #40609) Register the azure-eventhub input on Windows #40621

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade azure-eventhub to input v2 API #39511

Upgrade azure-eventhub to input v2 API #39511

zmoog commented May 10, 2024 •

edited

Loading

mergify bot commented May 10, 2024

elasticmachine commented May 22, 2024

belimawr left a comment •

edited

Loading

mergify bot commented May 24, 2024

constanca-m commented Jun 6, 2024

constanca-m Jun 6, 2024

zmoog Jun 10, 2024

mergify bot commented Jun 10, 2024

zmoog commented Jun 10, 2024

elasticmachine commented Jun 11, 2024

constanca-m commented Jun 11, 2024

cmacknz left a comment

	func init() {
	err := input.Register(inputName, NewInput)
	if err != nil {
	panic(fmt.Errorf("failed to register %v input: %w", inputName, err))
	}
	}

Upgrade azure-eventhub to input v2 API #39511

Upgrade azure-eventhub to input v2 API #39511

Conversation

zmoog commented May 10, 2024 • edited Loading

Proposed commit message

Notes for reviewers

Keep existing behavior: Beats pipeline acks

Checklist

Disruptive User Impact

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

mergify bot commented May 10, 2024

elasticmachine commented May 22, 2024

belimawr left a comment • edited Loading

Choose a reason for hiding this comment

mergify bot commented May 24, 2024

constanca-m commented Jun 6, 2024

constanca-m Jun 6, 2024

Choose a reason for hiding this comment

zmoog Jun 10, 2024

Choose a reason for hiding this comment

mergify bot commented Jun 10, 2024

zmoog commented Jun 10, 2024

elasticmachine commented Jun 11, 2024

constanca-m commented Jun 11, 2024

cmacknz left a comment

Choose a reason for hiding this comment

zmoog commented May 10, 2024 •

edited

Loading

belimawr left a comment •

edited

Loading