-
Notifications
You must be signed in to change notification settings - Fork 208
Add custom build support for lambdacomponents #1427
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
Conversation
I like this idea of making the life of user who want to extend the collector layer easier. However I'm not sure if we are ready to fully support this long term. I'm proposing to clearly state that this feature/characteristic of the repository is experimental for now and we highly advertise it so that users can start experimenting. WDYT @tylerbenson ? |
collector/lambdacomponents/custom.go
Outdated
receiverFactories []factory[receiver.Factory] | ||
processorFactories []factory[processor.Factory] | ||
exporterFactories []factory[exporter.Factory] | ||
extensionFactories []factory[extension.Factory] | ||
connectorFactories []factory[connector.Factory] |
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.
can we simplify this? can this be a list of functions (providers) that receive a extensionid as parameter and then return a factory as return value?
receiverFactories []func(extensionid string) receiver.Factory
processorFactories []func(extensionid string) processor.Factory
exporterFactories []func(extensionid string) exporter.Factory
extensionFactories []func(extensionid string) extension.Factory
connectorFactories []func(extensionid string) connector.Factory
Then in each init block in the modules you can use the AddReceiverFactoryProvider
& etc to append to the slices.
e.g.:
func init() {
lambdacomponents.AddReceiverFactoryProvider(func(extensionId string) receiver.Factory { return telemetryapireceiver.NewFactory(extensionID) })
}
then in Components
you just iterate over the elements invoking the provider to get the factories and pass that to the appropriate MakeFactoryMap
.
It think what you did is very nice but I prefer to keep the code simpler so that it is simpler to maintain and more welcoming for people trying to understand the code to extend the collector layer using your approach.
Thank you for your PR. This is a very neat idea. How would be the workflow for users who want to extend the collector layer? users will need to fork the repository and add code to instantiate new factories with new modules or is your idea to use the layer as a library somehow. Can you create a section in the README detailing how to extend the collector layer? We should make it clear that this is experimental. |
Hey, thanks for the feedback. I'm looking forward to implement it as suggested. |
Hey @rapphil , I have addressed your comments and rebased my fork. For the build to work, I had to move the definition of each of the Factory-Slices to each package, because otherwise there would've been a cyclic dependency. Please let me know if there's anything you'd like to see adjusted :) |
Collector binary sizes built with these commands:
|
README.md
Outdated
@@ -99,6 +101,45 @@ The following are runtimes which are no longer or not yet supported by this repo | |||
[3]: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/faas/faas-spans.md#outgoing-invocations | |||
[4]: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/faas/faas-metrics.md | |||
|
|||
## (Experimental) Customized collector build |
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.
These sections might be better in the collector/README.md
instead, especially since that documents how to publish the resulting layer.
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.
Moved the section to the collector README and updated the link in the main README accordingly
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.
Aside from moving the readme changes to the collector readme, I'm ok with this change.
collector/README.md
Outdated
|
||
would be the following: | ||
```shell | ||
go build -tags "lambdacomponents.custom,lambdacomponents.receiver.all,lambdacomponents.processor.all,lambdacomponents.exporter.otlphttp,lambdacomponents.connector.spanmetrics" |
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.
can we just add more details on how to build and publish this custom layer to the person's account?
we just need to include:
- add details about how to zip.
- how to publish. (the aws cli command that can be used).
I'm looking for end to end instructions in case someone not experienced with the process can follow. This will make the custom builds fully usable by anyone.
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.
Added this to the Makefile
making use of the env variable BUILDTAGS
and adjusted the README accordingly
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.
First things first, awesome feature and I identify with the motivation expressed in the PR. Here are a few comments:
My one requested change here is the addition of an ALTERNATIVES CONSIDERED section including consideration of employing the Collector’s ocb
for custom builds. I try to provide some helpful resources, including more detail on the custom build in the notes below for consideration.
See Juraci's article - https://medium.com/opentelemetry/building-your-own-opentelemetry-collector-distribution-42337e994b63 on custom buillds in the Collector.
I noticed in the PR message that imports were causing some issues. If I understand the issues you encountered correctly this problem of patching go.mod
with replace
statements can be resolved by developing with Go workspaces
, and some more about using workspaces for custom Collector development with the Collector is outlined in (the doc on building receivers)[https://opentelemetry.io/docs/collector/building/receiver/]. That's also a good introduction to the custom builder in practice.
Reiterating… awesome feature and I’m excited about the initiative and effort that’s brought us here.
@nslaughter I added the requested section to the main comment of this PR |
This PR looks great! LGTM. @nslaughter do you have any other concerns? |
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.
LGTM
Thanks for the diligence in updating changes.
Adding support for customized builds of the collector.
Motivation
I hoped to use the
spanmetricsconnector
using this collector, but sadly found it not to be part of this distribution.I also tried doing so in a customized fork, but found it to be extremely cumbersome to do with how go handles
internal
-packages together with forks. With how the project is structured, I saw myself adding a lot ofreplace
instructions to the differentgo.mod
files to make a successful build of my fork.I came to the conclusion that others might also want to use more specialized versions for their usecases and figured out this could be done without affecting the distribution for everyone else by making use of build tags.
Alternatives Considered
Customized fork of this repository
As mentioned above, I initially considered forking this repository and adding my desired components to my fork.
However, I faced issues building from the fork because of how go handles the
internal
-Package. I was able to solve the issues I was facing, but only by adding a lot ofreplace
instructions to severalgo.mod
files.This might be just due to my limited knowledge of go's build system.
Customized collector, utilizing this repositories collector as a library
Similar to https://medium.com/opentelemetry/building-your-own-opentelemetry-collector-distribution-42337e994b63
I tried building my own collector layer, utilizing the lambda-lifecycle of this project to integrate the resulting collector with AWS Lambdas extension API.
Trying to import the following packages:
Results in the following error on a fresh go project:
It seems not as easy to do this as I initially thought it would be.
Again, this might be just due to my limited knowledge of go's build system.
OCB
OCB works great for using a customized set of components not yet covered in any of the existing distributions or even adding fully custom componnents. However, this results in a "standard" collector which doesn't include the AWS Lambda extension API communication.
Conclusion
I checked previous PRs and Issues of this project and found that the usual approach was to add components (#102 , #1196) as long as these additional components are expected to be useful for a reasonable number of users of this specialized collector.
However, this approach does not work well for more niche components, which might only be useful for few usecases.
This PR adds an in-between method to include components, without inflating the collector binary with components which would only be useful for a minority.
Additionally, this PR enables everyone to reduce their self-built collector to only the set of components they actually need, potentially reducing the size of the resulting binary compared to the default set of components.
Usage
If the collector is built without build tags, the
default.go
Components will be used.If the collector is built with
lambdacomponents.custom
, thecustom.go
Components will be used.This allows a customized set of components to be included, for example:
go build -tags "lambdacomponents.custom,lambdacomponents.receiver.all,lambdacomponents.processor.all,lambdacomponents.exporter.otlphttp,lambdacomponents.connector.spanmetrics"
This would include: