-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
out_azure_logs_ingestion: Implementing Azure Logs Ingestion (with DCR, DCE) output plugin #7155
out_azure_logs_ingestion: Implementing Azure Logs Ingestion (with DCR, DCE) output plugin #7155
Conversation
de72285
to
286ef37
Compare
I would also like to see some integration tests to verify the feature in fluent/fluent-bit-ci. |
"client_credentials", 18); | ||
if (ret == -1) { | ||
flb_plg_error(ctx->ins, "error appending oauth2 params"); | ||
return NULL; |
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.
This exit path doesn't seem to release the mutex which could cause a deadlock.
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.
Fixed
sizeof(FLB_AZ_LI_AUTH_SCOPE) - 1); | ||
if (ret == -1) { | ||
flb_plg_error(ctx->ins, "error appending oauth2 params"); | ||
return NULL; |
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.
This exit path doesn't seem to release the mutex which could cause a deadlock.
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.
Fixed
ctx->client_id, -1); | ||
if (ret == -1) { | ||
flb_plg_error(ctx->ins, "error appending oauth2 params"); | ||
return NULL; |
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.
This exit path doesn't seem to release the mutex which could cause a deadlock.
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.
Fixed
ctx->client_secret, -1); | ||
if (ret == -1) { | ||
flb_plg_error(ctx->ins, "error appending oauth2 params"); | ||
return NULL; |
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.
This exit path doesn't seem to release the mutex which could cause a deadlock.
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.
Fixed
/* Copy string to prevent race conditions */ | ||
if (!token) { | ||
flb_plg_error(ctx->ins, "error retrieving oauth2 access token"); | ||
return NULL; |
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.
This exit path doesn't seem to release the mutex which could cause a deadlock.
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.
Fixed
ctx->token = flb_sds_create_size(token_len); | ||
if (!ctx->token) { | ||
flb_plg_error(ctx->ins, "error creating token buffer"); | ||
return NULL; |
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.
This exit path doesn't seem to release the mutex which could cause a deadlock.
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.
Fixed
} | ||
} | ||
/* If our token_length is more than what we already allocated */ | ||
else if (token_len > flb_sds_len(ctx->token)) { |
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.
You want to use flb_sds_alloc
here which gives you the size of the buffer rather than the length of the current value.
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.
Used flb_sds_alloc
/* If our token_length is more than what we already allocated */ | ||
else if (token_len > flb_sds_len(ctx->token)) { | ||
flb_plg_debug(ctx->ins, "new token len > previous token len"); | ||
ctx->token = flb_sds_increase(ctx->token, token_len - |
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.
You need to use a temporary variable here to save the result of flb_sds_increase
, otherwise if it fails when trying to resize the buffer you lose the original pointer and leak the memory.
Additionally, in case of failure you need to release the existing buffer and prematurely return, otherwise you will hit a null deref in line 230
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.
Fixed:
Simplified the codeblock by not using a struct variable since the token string must always persist for a HTTP call.
…gin. needs change Signed-off-by: Kushal Azim Ekram <[email protected]>
currently it builds with CMake Signed-off-by: Kushal Azim Ekram <[email protected]>
Signed-off-by: Kushal Azim Ekram <[email protected]>
Signed-off-by: Kushal Azim Ekram <[email protected]>
Signed-off-by: Kushal Azim Ekram <[email protected]>
Signed-off-by: Kushal Azim Ekram <[email protected]>
Signed-off-by: Kushal Azim Ekram <[email protected]>
Signed-off-by: Kushal Azim Ekram <[email protected]>
Signed-off-by: Kushal Azim Ekram <[email protected]>
Signed-off-by: Kushal Azim Ekram <[email protected]>
Signed-off-by: Kushal Azim Ekram <[email protected]>
Signed-off-by: Kushal Azim Ekram <[email protected]>
… Logs ingestion plugin Signed-off-by: Kushal Azim Ekram <[email protected]>
Signed-off-by: Kushal Azim Ekram <[email protected]>
Signed-off-by: Kushal Azim Ekram <[email protected]>
b60ae02
to
d380078
Compare
…, DCE) output plugin (fluent#7155) Signed-off-by: Kushal Azim Ekram <[email protected]>
…, DCE) output plugin (#7155) Signed-off-by: Kushal Azim Ekram <[email protected]>
@kforeverisback The Output plugin documentation is not available , Is that deleted ? https://docs.fluentbit.io/manual/pipeline/outputs/azure_logs_ingestion/ |
@infracloudav thanks for the highlight, it should be added now - the docs were there but not included in the summary to link to them. |
This PR implements an output plugin for Azure Log Ingestion API.
Issue:
Fixes #5222
Testing
Before we can approve your change; please submit the following in a comment:
Testing scenarios:
Documentation
Will try to include a documentation PR soon!See documentation PR: fluent/fluent-bit-docs#1076
Input Config and Valgrind Log
Config
Valgrind log
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.