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

Initial version of beatless #8437

Closed
wants to merge 15 commits into from
Closed

Conversation

ph
Copy link
Contributor

@ph ph commented Sep 25, 2018

This PR provide the following

  1. Plugin infra for developing providers and functions
  2. A local stdin provider only used for testing, I will remove it in the
    final version.
  3. AWS provider and function types for:
  • Cloudwatch logs
  • SQS
  • Kinesis
  • Api web gateway proxy
  1. License checker
  2. Packaging of artifact
  3. Runners
  4. CLI infrastructure
  5. CLI to push a cloudwatch logs function.
  6. CLI to delete any function
  7. Processors support.
  8. Types to validate value from the users and the lambda function.

What it doesn't provides

  • ECS and full event extraction. (NOT for v1)
  • Specifying the AWS credentials in the configuration
  • CLI for SQS, Kinesis, API
  • Robust CLI interaction with the API, rollback on failure / versioning.
  • Removal of not supported outputs
  • Removal of seccomp check
  • Integration tests
  • Updated build task to produce containing the user executable beat and
    the linux beats.
  • Concurrency / memory settings

Vendored

  • aws/aws-lambda-go
  • aws/aws-sdk-go-v2

Note: The PR look much bigger because of the vendored files.

Notes 2:

Make sure you add this to your config YAMl, I wan't to make the following the default options but it would have been really messy to do it in this PR.

# Disable anything that could be send to disk
path.data: /tmp
path.logs: /tmp/logs
logging.to_stderr: true
logging.to_files: false
#logging.level: debug
setup.template.enabled: true
queue.mem:
  events: 50
  flush.min_events: 1
  flush.timeout: 0.5s

related work

@ph ph added in progress Pull request is currently in progress. Functionbeat labels Sep 25, 2018
// feature are grouped by namespace, a namespace is a kind of plugin like outputs, inputs, or queue.
// The feature name must be unique.
type registry struct {
type FeatureRegistry struct {

Choose a reason for hiding this comment

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

type name will be used as feature.FeatureRegistry by other packages, and that stutters; consider calling this Registry

@@ -28,26 +28,26 @@ import (

type mapper map[string]map[string]Featurable

// Registry implements a global registry for any kind of feature in beats.
// Registry implements a global FeatureRegistry for any kind of feature in beats.

Choose a reason for hiding this comment

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

comment on exported type FeatureRegistry should be of the form "FeatureRegistry ..." (with optional leading article)

@ph ph requested a review from ruflin September 25, 2018 17:54
@ph
Copy link
Contributor Author

ph commented Sep 25, 2018

Forgot about the license check, working on fixing that.

@ph ph added review and removed in progress Pull request is currently in progress. labels Sep 26, 2018
@ph ph requested a review from kvch September 27, 2018 12:34

// TODO: Add registry reference here.
// Beatless is a beat designed to un under a serverless environment and listen to external triggers,
Copy link
Contributor

Choose a reason for hiding this comment

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

tyop: "to run"

// Beatless is a beat designed to un under a serverless environment and listen to external triggers,
// each invocation will generate one or more events to Elasticsearch.
//
// Each serverless implementation is different but beatless follows a few executions rules.
Copy link
Contributor

Choose a reason for hiding this comment

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

tyop: execution

return nil, fmt.Errorf("error reading config file: %v", err)
c := &config.DefaultConfig
if err := cfg.Unpack(c); err != nil {
return nil, fmt.Errorf("xerror reading config file: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

xerror? is this a typo?

return &Coordinator{log: log, runners: runners}
}

// Start starts each functions into an independant goroutine and wait until all the goroutine are
Copy link
Contributor

Choose a reason for hiding this comment

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

tyop: independent


// Start starts each functions into an independant goroutine and wait until all the goroutine are
// stopped to exit.
func (r *Coordinator) Start(ctx context.Context) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather name this function Run. Start implies there is a Stop function to call.

@ruflin
Copy link
Member

ruflin commented Sep 28, 2018

@ph One idea to make this more reviewable and not have to tell Github which files it should expand and which ones not: Could you open a PR with just the dependency against the feature branch and then rebase this one on top of it when it's merged?

)

// namespace is the namespace were providers will be registered in the global registry.
var namespace = "beatless.provider"
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be const.

// ns return the namespace for functions of a specific provider. The registry have a flat view
// representation of the plugin world this mean we don't really have a tree, instead what we do is
// to create a unique keys per providers that will only keep the functions of the provider.
func ns(provider string) string {
Copy link
Contributor

@kvch kvch Sep 28, 2018

Choose a reason for hiding this comment

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

Please rename this function to getNamespace or something more descriptive.

}

// defensive checks
if len(parsedEvent.LogEvents) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it invalid to get no events?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've looked at the SDK; this cannot happen the function should not be invoked in that case. I've removed the defensive check for all the function types.

@ph
Copy link
Contributor Author

ph commented Sep 28, 2018

@ruflin @kvch I've created #8483 with only the vendor changes lets get that merged and I will rebase this one.

This provide the following:

1. Plugin infra for developing providers and functions
2. A local stdin provider only used for testing, I will remove it in the
final version.
3. AWS provider and function types for:
  - Cloudwatch logs
  - SQS
  - Kinesis
  - Api web gateway proxy
4. License checker
5. Packaging of artifact
6. Runners
7. CLI infrastructure
8. CLI to push a cloudwatch logs function.
9. CLI to delete any function
10. Processors support.

What it doesn't provides:

- ECS and full event extraction.
- Specifying the AWS credentials in the configuration
- CLI for SQS, Kinesis, API
- Robust CLI interaction with the API, rollback on failure / versioning.
- Removal of not supported outputs
- Removal of seccomp check
- Integration tests
- Updated build task to produce containing the user executable beat and
the linux beats.

Vendored:

- https://github.com/aws/aws-lambda-go
- https://github.com/aws/aws-sdk-go-v2
When a project had two licenses, the script was creating two entries in
the NOTICE.txt but both entries would share the same license.

This commit also add the possibility to skip file that start with
LICENSE but doesn't contains any license information.

It also add MIT-0 license wording see https://github.com/aws/mit-0
@ph
Copy link
Contributor Author

ph commented Sep 28, 2018

Rebased + first round of comments fixed, I am waiting on merging #8483

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants