Skip to content
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

[azure logs] add routing integration to use only one azure-eventhub input #11984

Merged
merged 20 commits into from
Dec 9, 2024

Conversation

zmoog
Copy link
Contributor

@zmoog zmoog commented Dec 3, 2024

Proposed commit message

Switch the integration package from the one-input-per-data-stream model to the one-input model.

One input per data stream model:

image

One input model:

image

In the one-input model, there is only one azure-eventhub input running and sending events to the events data stream. In the events data stream, the ingest pipeline performs these tasks:

  • discover and set the event.dataset field using the category field in the event.
  • use the event.dataset field to reroute the event to the target data stream.

The discover process uses the following logic:

  • if the event doesn't have a category, it sets event.dataset to azure.eventhub (the generic integration)
  • if the event does have a category, it sets event.dataset to azure.platformlogs (it's probably an Azure log)
  • if the event category is supported, it sets event.dataset to specific one like azure.activitylogs or azure.signinlogs.

After the discovery step, the routing rules use the event.dataset value to forward the events to the best available target data stream.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • 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 practices

Author's Checklist

  • [ ]

How to test this PR locally

  • Bump the integration version
  • Build and start a local stack
  • Install the Azure Logs package (turn on v2 and turn off all v1s)
  • Send test documents and check they are redirected to the data stream

Bump the integration version

I'll update the version and changelog later to avoid conflicts. In the meantime, a simple bump would do:

diff --git a/packages/azure/changelog.yml b/packages/azure/changelog.yml
index bd20d10043..6da52b0424 100644
--- a/packages/azure/changelog.yml
+++ b/packages/azure/changelog.yml
@@ -1,3 +1,8 @@
+- version: "1.20.0+build18"
+  changes:
+    - description: Add Azure Logs integration v2 (preview)
+      type: enhancement
+      link: https://github.com/elastic/integrations/pull/11432
 - version: "1.19.4"
   changes:
     - description: Fix destination.geo.region_name mapping.
diff --git a/packages/azure/manifest.yml b/packages/azure/manifest.yml
index be7dccf294..895d9a8b6e 100644
--- a/packages/azure/manifest.yml
+++ b/packages/azure/manifest.yml
@@ -1,6 +1,6 @@
 name: azure
 title: Azure Logs
-version: 1.19.4
+version: "1.20.0+build18"
 description: This Elastic integration collects logs from Azure
 type: integration
 icons:

Build and start a local stack

elastic-package build && elastic-package stack up -d -v --version 8.16.1

Install the Azure Logs package

  • Turn on the v2 preview integration
  • Turn off all the v1 integrations

Send test documents and check they are redirected to the data stream

I use the eventhubs CLI tool to send JSON documents to an event hub.

Set up some environment variables:

export EVENTHUB_CONNECTION_STRING="[redacted]"
export EVENTHUB_NAMESPACE="[redacted]"
export EVENTHUB_NAME="logs"

We can then cat the test document and pipe them into an event hub using eh -v eventdata send-batch:

cat data_stream/provisioning/_dev/test/pipeline/test-provisioninglogs-raw.log | eh -v eventdata send-batch 

All supported documents should land into the expected data stream. Any unsupported document should land into the platform logs data stream.

Related issues

Screenshots

Integration setup

CleanShot 2024-12-03 at 15 46 55@2x

Sample from the integration docs

CleanShot 2024-12-03 at 23 25 35@2x

@elasticmachine
Copy link

elasticmachine commented Dec 3, 2024

💔 Build Failed

Failed CI Steps

History

@andrewkroh andrewkroh added the Integration:azure Azure Logs label Dec 3, 2024
@zmoog
Copy link
Contributor Author

zmoog commented Dec 3, 2024

As the first step, this PR introduces a v2 integration that can handle all the Azure Logs using one input only.

  • We mark the new integration as "v2" while all the existing integrations are marked as "v1".
  • We renamed the generic event hub integration "Collect raw events (v1)" and recommend using the Custom Azure Logs integration for more flexibility.

CleanShot 2024-12-03 at 15 46 55@2x

@zmoog zmoog self-assigned this Dec 3, 2024
@zmoog zmoog added Team:obs-ds-hosted-services Label for the Observability Hosted Services team [elastic/obs-ds-hosted-services] enhancement New feature or request labels Dec 3, 2024
zmoog added 7 commits December 3, 2024 16:12
It doesn't make sense in this data stream.
Used only when users do not provide a custom storage_account_container
name.
I am starting from the v1 docs, and I will make the changes needed
for v2.
@zmoog zmoog marked this pull request as ready for review December 3, 2024 22:13
@zmoog zmoog requested review from a team as code owners December 3, 2024 22:13
@elastic-vault-github-plugin-prod

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@zmoog zmoog changed the title [azure logs] add routing integration to use only one azure-eventhub input (WIP) [azure logs] add routing integration to use only one azure-eventhub input Dec 3, 2024
Copy link
Contributor

@alaudazzi alaudazzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the packages/azure/docs/events.md file and left a few minor editing suggestions.

packages/azure/docs/events.md Outdated Show resolved Hide resolved
packages/azure/docs/events.md Outdated Show resolved Hide resolved
packages/azure/docs/events.md Outdated Show resolved Hide resolved

If you need to collect raw events from Azure Event Hub, we recommend using the [Custom Azure Logs integration](https://www.elastic.co/docs/current/integrations/azure_logs) which provides more flexibility.

To learn more about the efficiency and routing enhancements introduced in version 1.20.0, please read the [Azure Logs (v2 preview)](https://www.elastic.co/docs/current/integrations/azure/events) documentation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the events.md file that will be rendered correct? Just the word efficiency made me read again the evetns.md and was trying to find a comparison between v1 vs v2.
Maybe just general, To learn more about the enhancements

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the events.md file that will be rendered correct?

Yes, the events.md file will eventually be available at https://www.elastic.co/guide/en/integrations/current/azure-events.html (now it's a 404).

Maybe just general, "To learn more about the enhancements"

You mean adding a link to the GitHub issues with more details, or add a sections that explains it? Should we extend the "What's new in v2 preview?" section?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both can work. I would add some more details why v2 is better (if you have not done it already in last updates)

- target_dataset: azure.provisioning
if: ctx.event?.dataset == 'azure.provisioning'
namespace:
- "{{data_stream.namespace}}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that you want to have also the additonal namespaces defined in the policy, along with default?

Should not then have a field namespace in manifest like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm, good question. I don't know how it works under the hood, but the integration seems to manage data_stream.* its own.

@gizas
Copy link
Contributor

gizas commented Dec 5, 2024

I run some tests and all work fine @zmoog !

Some samples

Screenshot 2024-12-05 at 5 43 55 PM

Did not manage to run the eh tool by the way with following error

env | grep EVENTHUB
EVENTHUB_CONNECTION_STRING=Endpoint=sb://gizas-test.servicebus.windows.net/;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey= ....
EVENTHUB_NAME=activitylogs
❯ cat  data_stream/provisioning/_dev/test/pipeline/test-provisioninglogs-raw.log | eh -v eventdata send-batch
Sending 3 events to activitylogs
Traceback (most recent call last):
  File "/opt/homebrew/bin/eh", line 8, in <module>
    sys.exit(cli())
             ~~~^^
  File "/opt/homebrew/lib/python3.13/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.13/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
  File "/opt/homebrew/lib/python3.13/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^
  File "/opt/homebrew/lib/python3.13/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^
  File "/opt/homebrew/lib/python3.13/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.13/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
  File "/opt/homebrew/lib/python3.13/site-packages/click/decorators.py", line 33, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/opt/homebrew/lib/python3.13/site-packages/eventhubs/cli.py", line 171, in send_batch
    batch = producer.create_batch()
  File "/opt/homebrew/lib/python3.13/site-packages/azure/eventhub/_producer_client.py", line 740, in create_batch
    self._get_max_message_size()
    ~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/opt/homebrew/lib/python3.13/site-packages/azure/eventhub/_producer_client.py", line 336, in _get_max_message_size
    )._open_with_retry()
      ~~~~~~~~~~~~~~~~^^
  File "/opt/homebrew/lib/python3.13/site-packages/azure/eventhub/_producer.py", line 155, in _open_with_retry
    return self._do_retryable_operation(self._open, operation_need_param=False)
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.13/site-packages/azure/eventhub/_client_base.py", line 608, in _do_retryable_operation
    raise last_exception from None
azure.eventhub.exceptions.AuthenticationError: CBS Token authentication failed.
Status code: None
Error: client-error
CBS Token authentication failed.
Status code: None

While buiding the package I see the following:

2024/12/05 16:38:32  INFO Skipped errors: found 5 validation errors:
   1. references found in dashboard kibana/dashboard/azure-1e5c9b50-f24a-11ec-a5a8-bf965bcd5646.json: azure-671ff040-f24e-11ec-a5a8-bf965bcd5646 (search) (SVR00004)
   2. references found in dashboard kibana/dashboard/azure-280493a0-f1a1-11ec-a5a8-bf965bcd5646.json: azure-fb61c4c0-f1a1-11ec-a5a8-bf965bcd5646 (search) (SVR00004)
   3. references found in dashboard kibana/dashboard/azure-8731b980-f1aa-11ec-a5a8-bf965bcd5646.json: azure-252228a0-f1ab-11ec-a5a8-bf965bcd5646 (search) (SVR00004)
   4. references found in dashboard kibana/dashboard/azure-91224490-f1a6-11ec-a5a8-bf965bcd5646.json: azure-70cbce40-f1a7-11ec-a5a8-bf965bcd5646 (search) (SVR00004)
   5. references found in dashboard kibana/dashboard/azure-cad82b40-f251-11ec-a5a8-bf965bcd5646.json: azure-3d1466b0-f252-11ec-a5a8-bf965bcd5646 (search) (SVR00004)

(although I see the - SVR00004 # references found in dashboard in validation. Not sure if you need those)

Probably you can delete such reference from dashboards if not used

Approving as rest looks good and please have a look mainly in #11984 (comment)

@gizas
Copy link
Contributor

gizas commented Dec 5, 2024

Some minors:

  • format_version: "3.0.2" --> to 3.3.0 probably and retest?
  • conditions.kibana.version: "^8.13.0" ---> Not sure when routing_rules introduced, is that accurate?

@muthu-mps
Copy link
Contributor

muthu-mps commented Dec 6, 2024

@zmoog - What if a user enables V2 and did not disable the V1? Do you think whats new added in the config description can go in the document and we say more explicitly to disable V1 while enabling V2 on top of the config description. WDYT?

@zmoog
Copy link
Contributor Author

zmoog commented Dec 6, 2024

Probably you can delete such reference from dashboards if not used

Yeah, this is something we need to address. I'll open an issue if I can't fix them quickly in this PR.

@zmoog
Copy link
Contributor Author

zmoog commented Dec 6, 2024

What if a user enables V2 and did not disable the V1?

They may get duplicate events and increased contention among consumers.

Do you think whats new added in the config description can go in the document and we say more explicitly to disable V1 while enabling V2 on top of the config description. WDYT?

Yeah, I agree. While reading the text today, I noticed the text is not clear and we should be more explicit. Let me share the updated version...

@zmoog
Copy link
Contributor Author

zmoog commented Dec 6, 2024

Do you think whats new added in the config description can go in the document and we say more explicitly to disable V1 while enabling V2 on top of the config description. WDYT?

What about this version?

CleanShot 2024-12-06 at 12 46 10@2x

@zmoog
Copy link
Contributor Author

zmoog commented Dec 6, 2024

Some minors:

  • format_version: "3.0.2" --> to 3.3.0 probably and retest?

Oh, great point, I missed it! 🤦 — thank you for the heads up, I'm updating the package-spec version and I'll re-run the tests.

  • conditions.kibana.version: "^8.13.0" ---> Not sure when routing_rules introduced, is that accurate?

Stack version 8.13.0 is okay:

  • Elasticsearch introduced the reroute processor in release 8.8.0.
  • package-spec added support for the routing rules in version v2.9.0

We finally found the time to update the integration with these long awaited changes.

Copy link
Contributor

@muthu-mps muthu-mps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

We bump it to 1.20.0 because the v2 integration is in preview.
@zmoog zmoog enabled auto-merge (squash) December 9, 2024 10:31
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

@zmoog zmoog merged commit f99850b into elastic:main Dec 9, 2024
4 of 5 checks passed
@elasticmachine
Copy link

💚 Build Succeeded

History

cc @zmoog

@elastic-vault-github-plugin-prod

Package azure - 1.20.0 containing this change is available at https://epr.elastic.co/package/azure/1.20.0/

@zmoog zmoog deleted the zmoog/one-input-to-rule-them-all-2 branch December 9, 2024 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Integration:azure Azure Logs Team:obs-ds-hosted-services Label for the Observability Hosted Services team [elastic/obs-ds-hosted-services]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Azure Logs: use one input per agent policy
6 participants