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

Fix Python Sample App dep and collector extension names #701

Closed
wants to merge 2 commits into from

Conversation

vasireddy99
Copy link
Contributor

@vasireddy99 vasireddy99 commented May 18, 2023

This PR fixes the python sample app, so it can pick up the right boto3 package versions that compatible with urlib3 and to solve the following error.

image

Also fixes the naming conventions of collector extension layer.

@vasireddy99 vasireddy99 changed the title Fix Python Sample App and extension names Fix Python Sample App dep and collector extension names May 19, 2023
@@ -48,12 +48,12 @@ package: build

.PHONY: publish
publish:
aws lambda publish-layer-version --layer-name $(LAYER_NAME) --zip-file fileb://$(BUILD_SPACE)/collector-extension-$(GOARCH).zip --compatible-runtimes nodejs14.x nodejs16.x nodejs18.x java11 python3.8 python3.9 --query 'LayerVersionArn' --output text
aws lambda publish-layer-version --layer-name $(LAYER_NAME) --zip-file fileb://$(BUILD_SPACE)/opentelemetry-collector-layer-$(GOARCH).zip --compatible-runtimes nodejs14.x nodejs16.x nodejs18.x java11 python3.8 python3.9 --query 'LayerVersionArn' --output text
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that the name for the file produced in the .PHONY: package is opentelemetry-collector-layer-$(GOARCH).zip. Is this fixing an issue where the .PHONY: publish action was failing because it couldn't find the file to publish? 🤔

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 That is correct and it will publish now with this change, it wasn't able to find the file since it was renamed.


.PHONY: publish-layer
publish-layer: package
@echo Publishing collector extension layer...
aws lambda publish-layer-version --layer-name $(LAYER_NAME) --zip-file fileb://$(BUILD_SPACE)/collector-extension-$(GOARCH).zip --compatible-runtimes nodejs14.x nodejs16.x nodejs18.x java11 python3.8 python3.9 --query 'LayerVersionArn' --output text
aws lambda publish-layer-version --layer-name $(LAYER_NAME) --zip-file fileb://$(BUILD_SPACE)/opentelemetry-collector-layer-$(GOARCH).zip --compatible-runtimes nodejs14.x nodejs16.x nodejs18.x java11 python3.8 python3.9 --query 'LayerVersionArn' --output text
Copy link
Contributor

Choose a reason for hiding this comment

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

I found collector-extension-$(GOARCH).zip elsewhere in this repo:

tigre@hilleman:~/github/ocelotl/opentelemetry-lambda$ ack collector-extension
go/integration-tests/aws-sdk/wrapper/main.tf
4:  filename            = "${path.module}/../../../../collector/build/collector-extension-amd64.zip"
7:  source_code_hash    = filebase64sha256("${path.module}/../../../../collector/build/collector-extension-amd64.zip")

python/integration-tests/aws-sdk/wrapper/main.tf
12:  filename            = "${path.module}/../../../../collector/build/collector-extension-amd64.zip"
15:  source_code_hash    = filebase64sha256("${path.module}/../../../../collector/build/collector-extension-amd64.zip")

nodejs/integration-tests/aws-sdk/wrapper/main.tf
12:  filename            = "${path.module}/../../../../collector/build/collector-extension-amd64.zip"
15:  source_code_hash    = filebase64sha256("${path.module}/../../../../collector/build/collector-extension-amd64.zip")

collector/Makefile
51:     aws lambda publish-layer-version --layer-name $(LAYER_NAME) --zip-file fileb://$(BUILD_SPACE)/collector-extension-$(GOARCH).zip --compatible-runtimes nodejs14.x nodejs16.x nodejs18.x java11 python3.8 python3.9 --query 'LayerVersionArn' --output text
56:     aws lambda publish-layer-version --layer-name $(LAYER_NAME) --zip-file fileb://$(BUILD_SPACE)/collector-extension-$(GOARCH).zip --compatible-runtimes nodejs14.x nodejs16.x nodejs18.x java11 python3.8 python3.9 --query 'LayerVersionArn' --output text

java/integration-tests/okhttp/wrapper/main.tf
12:  filename            = "${path.module}/../../../../collector/build/collector-extension-amd64.zip"
15:  source_code_hash    = filebase64sha256("${path.module}/../../../../collector/build/collector-extension-amd64.zip")

java/integration-tests/aws-sdk/wrapper/main.tf
12:  filename            = "${path.module}/../../../../collector/build/collector-extension-amd64.zip"
15:  source_code_hash    = filebase64sha256("${path.module}/../../../../collector/build/collector-extension-amd64.zip")

java/integration-tests/aws-sdk/agent/main.tf
12:  filename            = "${path.module}/../../../../collector/build/collector-extension-amd64.zip"
15:  source_code_hash    = filebase64sha256("${path.module}/../../../../collector/build/collector-extension-amd64.zip")

dotnet/integration-tests/aws-sdk/wrapper/main.tf
4:  filename            = "${path.module}/../../../../collector/build/collector-extension-amd64.zip"
7:  source_code_hash    = filebase64sha256("${path.module}/../../../../collector/build/collector-extension-amd64.zip")

utils/sam/run.sh
95:             rm -f build/collector-extension-amd64.zip

do we need to also change those other occurrences?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The rest of the occurrences in the .tf files are invalid for use in this repo. I am going to PR soon to remove those unused files in this repo.

@vasireddy99 vasireddy99 requested a review from ocelotl May 25, 2023 07:27
@vasireddy99
Copy link
Contributor Author

@ocelotl I have included these changes in this PR #717 to fix holistically. Please review when you get a chance.

closing this PR here.

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.

2 participants