Skip to content

[Workflow] Fix Image Tagging for GitHub Workflow for Python Sample App ECR Upload #40

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

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

jj22ee
Copy link
Contributor

@jj22ee jj22ee commented Apr 16, 2024

Issue #, if available:
Workflow to update Python Sample App in ECR failed - https://github.com/aws-observability/aws-application-signals-test-framework/actions/runs/8697504333/job/23852856554

Error parsing reference: "***.dkr.ecr.af-south-1.amazonaws.com/***:latest" is not a valid repository/tag: invalid reference format

Cause: latest tag is already within the GitHub secret, so the :latest is duplicated.

Description of changes:
Remove duplicate :latest tag.

Testing:
Successful upload with change - https://github.com/aws-observability/aws-application-signals-test-framework/actions/runs/8697577697/job/23853061967

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

@jj22ee jj22ee requested review from srprash and zzhlogin April 16, 2024 00:02
@jj22ee jj22ee merged commit d458c41 into main Apr 16, 2024
@jj22ee jj22ee deleted the python-ecr-upload-fix branch April 16, 2024 00:19
zzhlogin pushed a commit to zzhlogin/aws-application-signals-test-framework that referenced this pull request Jun 6, 2024
…-ecr-upload-fix

[Workflow] Fix Image Tagging for GitHub Workflow for Python Sample App ECR Upload
zzhlogin pushed a commit to zzhlogin/aws-application-signals-test-framework that referenced this pull request Jun 6, 2024
…-ecr-upload-fix

[Workflow] Fix Image Tagging for GitHub Workflow for Python Sample App ECR Upload
zzhlogin pushed a commit to zzhlogin/aws-application-signals-test-framework that referenced this pull request Jun 6, 2024
…-ecr-upload-fix

[Workflow] Fix Image Tagging for GitHub Workflow for Python Sample App ECR Upload
zzhlogin pushed a commit to zzhlogin/aws-application-signals-test-framework that referenced this pull request Jun 6, 2024
…-ecr-upload-fix

[Workflow] Fix Image Tagging for GitHub Workflow for Python Sample App ECR Upload
georgeboc pushed a commit to georgeboc/aws-application-signals-test-framework that referenced this pull request Jul 8, 2024
In this commit, we are setting up contract tests similar to those in the aws-otel-java-instrumentation repo. Initially, we are just committing the logic to run the mock collector and a single application that uses the requests library, but the tests are extensible to other frameworks.

There are a number of small differences between these tests and the Java contract tests, most of which are language dependent:
* I've disabled code style checks on three files: `mock_collector_service_pb2_grpc.py`, `mock_collector_service_pb2.py`, and`mock_collector_service_pb2.pyi`. These are generated GRPC files and no edits to these files are recommended after generating from proto files. I have made no edits to these files after generating with commands described in the mock collector README.
* `requests_server.py` is based on the [`native-http-client/App.java`](https://github.com/aws-observability/aws-otel-java-instrumentation/blob/main/appsignals-tests/images/http-clients/native-http-client/src/main/java/com/amazon/sampleapp/App.java)
* `mock_collector_client.py` is based on the [`MockCollectorClient.java`](https://github.com/aws-observability/aws-otel-java-instrumentation/blob/main/appsignals-tests/contract-tests/src/test/java/software/amazon/opentelemetry/appsignals/test/utils/MockCollectorClient.java) and [`ResourceScopeSignal.kt`](https://github.com/aws-observability/aws-otel-java-instrumentation/blob/main/appsignals-tests/contract-tests/src/test/kotlin/software/amazon/opentelemetry/appsignals/test/utils/ResourceScopeSignal.kt). Note that a lot of the serialization/deserialization logic is simplified in Python/gRPC.
* `mock-collector` classes (e.g. `mock_collector_server`, `mock_collector_metrics_service`, etc are based on the [`mockcollector` java classes](https://github.com/aws-observability/aws-otel-java-instrumentation/tree/main/appsignals-tests/images/mock-collector/src/main/java/software/amazon/opentelemetry/appsignals/test/images/mockcollector). Callout that java uses an HTTP server wrapping gRPC servicers, while I'm just using a gRPC server and servicers as it was simpler to do so in Python.
* `contract_test_base.py` is based on [`ContractTestBase.java`](https://github.com/aws-observability/aws-otel-java-instrumentation/blob/main/appsignals-tests/contract-tests/src/test/java/software/amazon/opentelemetry/appsignals/test/base/ContractTestBase.java). One substantial difference is that we are not passing information in
about the agent, since both testcontainers and OTEL instrumentation mechanisms are quite different in Java vs Python.
* `requests_test.py` is based on [`BaseHttpClientTest.java`](https://github.com/aws-observability/aws-otel-java-instrumentation/blob/main/appsignals-tests/contract-tests/src/test/java/software/amazon/opentelemetry/appsignals/test/httpclients/base/BaseHttpClientTest.java) and [`NativeHttpClientTest.java`](https://github.com/aws-observability/aws-otel-java-instrumentation/blob/main/appsignals-tests/contract-tests/src/test/java/software/amazon/opentelemetry/appsignals/test/httpclients/nativehttpclient/NativeHttpClientTest.java)
   * Note that `PEER_SERVICE` does not appear to be supported by Python
* Note that OTEL does not instrument basic HTTP Servers, so there are no server spans produced by the application and only client spans are produced `requests`. This is acceptable from a contract perspective, but does result in `AWS_LOCAL_OPERATION` being `InternalOperation` and `AWS_SPAN_KIND` being `LOCAL_ROOT` on spans/metrics (in metrics `AWS_SPAN_KIND` is `CLIENT`, this is expected)
* Note that `NET_PEER_NAME` and `NET_PEER_PORT` are not populated by `requests` instrumentation, which appears to be an upstream bug. This results in `AWS_REMOTE_SERVICE` being `UnknownRemoteService`
* `pylint: disable` summary:
   * `invalid-name` - only used where I do not have control over the method name (e.g. overrides)
   * `broad-exception-caught` - Used where we really do want to just catch everything and keep going
   * `no-self-use` - only used when defining methods that are designed to be overridden by subclasses.
   * `no-member` - Used for `Span.SPAN_KIND_CLIENT` - for some reason, pylint cannot detect this constant, likely related to gRPC/proto magic

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
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