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

Fluent Bit Lambda Extensions Example added #42

Closed
wants to merge 2 commits into from

Conversation

youngjeong46
Copy link

Issue #, if available:

Description of changes:

Added extensions example for Fluent-Bit. Requesting Code Review.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Comment on lines +54 to +58
// Determines listener port, use default 1234 if not provided as Lambda environment variable
listenerPort := os.Getenv("PORT")
if os.Getenv("PORT") == "" {
listenerPort = defaultListenerPort
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to use another variable name rather than the generic PORT. All extensions and the function share ENVVARS so perhaps making it more specific to the extension with something such as FLB_PORT would be preferable

)

const (
defaultListenerPort = "1234" // DefaultListenerPort sets the http port to 1234 if not provided by function env variable "HOST"
Copy link
Contributor

@julianwood julianwood Apr 8, 2021

Choose a reason for hiding this comment

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

This isn't the variable, should be "PORT" or changed based on other suggestion.

Copy link
Contributor

@julianwood julianwood left a comment

Choose a reason for hiding this comment

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

Thanks for the request, just a ENVVAR naming suggestion.

logger.Fatal("The DNS name sandbox cannot be resolved.")
}
host := addr[0]
err := os.Setenv("HOST", host)
Copy link
Contributor

Choose a reason for hiding this comment

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

FLB_HOST

defaultListenerPort = "1234" // DefaultListenerPort sets the http port to 1234 if not provided by function env variable "HOST"
maxItems = 10000
maxBytes = 262144
timeOut = 1000
Copy link
Contributor

Choose a reason for hiding this comment

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

Reduce this

@@ -0,0 +1,87 @@
# Fluent Bit Lambda Extensions in Go

Choose a reason for hiding this comment

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

nit: Fluent Bit Logs Extension for Lambda


## Prerequisites

1. You need to compile a Fluent Bit binary. The binary should be placed inside the `fluent-bit/` directory. Here are the steps:

Choose a reason for hiding this comment

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

Can we include the binary you have been testing with in this repo? That way users can run the example without this step. You can reword it to say that its to update the binary to a new version.

Comment on lines +9 to +10
Listen ${HOST}
Port ${PORT}

Choose a reason for hiding this comment

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

this seems to be missing the new tag_key field that I added

The provided code sample demonstrates how to get Fluent Bit set up for Lambda Extensions.

Note:
1. Supported Fluent Bit: v1.6

Choose a reason for hiding this comment

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

This is not true. My code changes have not been merged yet, but once they are, it will mean Lambda is supported in Fluent Bit version 1.7.4 or greater

Choose a reason for hiding this comment

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

Also, link to the official Fluent Bit docs somewhere in the beginning

@r0mdau
Copy link

r0mdau commented Feb 19, 2022

Thanks for this example.

Have you planned (or discussed with fluentbit maintainers) to develop directly a input plugin in fluentbit that implement the lambda logs API ?

@PettitWesley
Copy link

PettitWesley commented Apr 5, 2022

NOTE: For folks who find this and wonder why we abandoned it, it was because the extension tended to lose logs when the lambda function was invoked repeatedly with a very short runtime. It is possible that this solution can work well if you know that your function will always execute for at least 1 second.

@r0mdau We discussed building native support in FB but that would require a lot more work. It's very non-trivial.

@julianwood
Copy link
Contributor

Closing based on submitter comments.

@julianwood julianwood closed this Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants