-
Notifications
You must be signed in to change notification settings - Fork 20
[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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zzhlogin
reviewed
Apr 16, 2024
zzhlogin
approved these changes
Apr 16, 2024
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
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.