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

Adding ruby lambda layer #918

Merged
merged 16 commits into from
Jan 26, 2024
Merged

Conversation

xuan-cao-swi
Copy link
Contributor

Description

Adding the simple ruby lambda layer. For try-out and test run, please see the instruction under README.md.

There are two main directories:

  • ruby/src/ directory contains files to build the actual layer. Notably, the zip_ruby_layer.sh is a helper bash script designed to zip the Ruby gems folder.
  • ruby/sample-apps/ contains a test application that can be run locally, allowing users to verify if the layer truly incorporates the Ruby OpenTelemetry component.

With respect to GitHub Actions, the workflow for ruby to push layer is defined in release-layer-ruby.yml. However, it still requires testing with a valid ACTIONS_RUNTIME_TOKEN and testing that push the layer to AWS Lambda. Both dependabot.yml and codeql.yml have been updated to support Ruby.

Co-authored-by: Tristan Sloughter <[email protected]>
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@xuan-cao-swi xuan-cao-swi requested a review from a team October 26, 2023 18:04
Copy link
Member

@tsloughter tsloughter left a comment

Choose a reason for hiding this comment

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

I think this is good as a first step.

There will be follow up needed, by someone, to add an instrumentation library to ruby-contrib and then to use that library here in a new otel_wrapper.rb that will setup the instrumentation and create the span.

Copy link
Member

@mwear mwear left a comment

Choose a reason for hiding this comment

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

LGTM

@tylerbenson
Copy link
Member

@xuan-cao-swi are you willing to help maintain this going forward? We don't have anyone familiar with Ruby actively participating in the SIG right now.


```bash
cd .aws-sam/build/OTelLayer/
zip -qr ../../../<your_layer_name>.zip ruby/

Choose a reason for hiding this comment

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

With this example, would the <your_layer_name> value always be OTelLayer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, because sam takes the template.yaml file to create folder. However, people can change the name if they change the template.yaml file in L10 (e.g. change from OTelLayer to <other_name>), then the folder name will change to <other_name>

@xuan-cao-swi
Copy link
Contributor Author

@xuan-cao-swi are you willing to help maintain this going forward? We don't have anyone familiar with Ruby actively participating in the SIG right now.

Yes, I am willing to maintain this layer. If there is a SIG talk about lambda progress, I am also happy to participate.

@tylerbenson
Copy link
Member

@xuan-cao-swi We have bi-weekly SIG meetings on Tuesdays that you are welcome to attend. Also, please join the #otel-faas slack channel.

Copy link
Member

@tylerbenson tylerbenson left a comment

Choose a reason for hiding this comment

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

I'm ok approving this but I also want to let @rapphil chime in.

@rapphil
Copy link
Contributor

rapphil commented Jan 23, 2024

the code LGTM. my only concern is the maintenance of this layer in the long term. Since @xuan-cao-swi is willing to help maintaining it, I'm ok with accepting it.

Thank you very much for your contribution.

@tylerbenson
Copy link
Member

Ok, I'm going to merge. Please reply here with the outstanding tasks needed before we can publish this as a layer.

@tylerbenson tylerbenson merged commit 4471945 into open-telemetry:main Jan 26, 2024
@xuan-cao-swi
Copy link
Contributor Author

Ok, I'm going to merge. Please reply here with the outstanding tasks needed before we can publish this as a layer.

Thanks, I kind hope open-telemetry/opentelemetry-ruby-contrib#721 get merge then we can create a complete layer.

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.

6 participants