-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(azure_logs_ingestion sink): Initial azure_logs_ingestion sink
#22912
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
base: master
Are you sure you want to change the base?
feat(azure_logs_ingestion sink): Initial azure_logs_ingestion sink
#22912
Conversation
Signed-off-by: Jed Laundry <[email protected]>
Signed-off-by: Jed Laundry <[email protected]>
Signed-off-by: Jed Laundry <[email protected]>
Signed-off-by: Jed Laundry <[email protected]>
Signed-off-by: Jed Laundry <[email protected]>
Signed-off-by: Jed Laundry <[email protected]>
Signed-off-by: Jed Laundry <[email protected]>
Signed-off-by: Jed Laundry <[email protected]>
Signed-off-by: Jed Laundry <[email protected]>
Signed-off-by: Jed Laundry <[email protected]>
Signed-off-by: Jed Laundry <[email protected]>
Signed-off-by: Jed Laundry <[email protected]>
check-spelling run (pull_request_target) for feature-azure_logs_ingestion Signed-off-by: check-spelling-bot <[email protected]> on-behalf-of: @check-spelling <[email protected]>
pront
left a comment
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.
Hi @jlaundry, thanks for this contribution!
It looks good. Two things:
- We will need some documentation files. See an example here (all files under
website). Note thatbase/is generated bymake generate-component-docs. - Is the intention to complete replace the
azure_monitor_logssink? If that's the case maybe we can mark the existing one as deprecated in favor of this new sink.
rtrieu
left a comment
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.
Thank you for your contribution! Approving with one very minor suggestion.
website/cue/reference/components/sinks/base/azure_logs_ingestion.cue
Outdated
Show resolved
Hide resolved
…on.cue Co-authored-by: Rosa Trieu <[email protected]>
Signed-off-by: Jed Laundry <[email protected]>
azure_logs_ingestion sinkazure_logs_ingestion sink
Signed-off-by: Jed Laundry <[email protected]>
From memory, there is a minor refactor required in https://github.com/jlaundry/vector/blob/02562be6447af36404d8b5668434e317c87a45b2/src/sinks/azure_logs_ingestion/config.rs#L139 to change it back to Probably easiest if you rollback the package first, and then I'll rebase, retest, and push. |
|
FYI - Reverted deps #23039 |
|
FYI, I started preparing the rebase, but I've seen that the Azure Rust team have recently decided to change how authentication works with the Depending on what they decide, we may need to explicitly configure credentials, either via the vector config file or environment variables. The So instead of releasing an initial sink, and then requiring another config/environment change, I'll wait until there's stability in the SDK before moving forward with this PR. (I'm not giving up!!!) |
|
Hi @jlaundry, By the way, I see that the discussion here is closed. Does it mean you will go forward with merging your PR? Thanks a lot! |
Makes sense. I feel that these crates are unfortunately a bit unstable and each version update is risky. Thank you for your interest in contributing 👍 |
|
Hi all, currently the |
1720078 to
ffe54be
Compare
|
For those playing along at home (hi @yoelk @Renizmy), a summary of the current issues and why we're blocked:
What I think this means for this PR, and my thoughts/opinions for the wider project:
Also note: The existing |
Hi @jlaundry, I missed the context on this one. How does this break existing users? E.g. assume we keep the |
@pront the current documented behavior of the
This is based on the
Based on past experience with customers, I expect 60-70% of production users are using So yes, I support removing the |
|
Very nice work. Looking forward to seeing this upstream. Right now I am using Logstash with the |
|
Hi. Can we expect this to be included in the next release of Vector? |
Hi @sb1-nicolai, this depends on @jlaundry and the community. The Vector team is not actively working on this PR. |
|
While I am still keen to finish this feature in time for the older API deprecation, I don't want to load the project with an unmanageable/unsupported mess. Unfortunately, not much has changed since I wrote #22912 (comment) The And on the @sb1-nicolai if Azure Log Ingestion is important to you and your team, may I please suggest you reach out to your Microsoft CSAM, and ask them to escalate to the product group. Otherwise... Vector has fantastic support for other cloud logging platforms... 😉 |
Summary
The current
azure_monitor_logssink uses the Data Collector API, which has been deprecated and will be removed in September 2026.This sink uses the replacement Logs Ingestion API.
While I did consider making this a drop-in replacement for the existing sink, users need to make numerous breaking infrastructure changes, including:
_CLcustom tables.Change Type
Is this a breaking change?
How did you test this PR?
AZURE_TENANT_ID,AZURE_CLIENT_ID, andAZURE_CLIENT_SECRETenvironment variables from the App Registrationvector.yaml:Does this PR include user facing changes?
References
azure_blob, by using the Azure Identity crate, this sink supports passwordless credentials.