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

Add E2E coverage for Trace and Logging #199

Merged
merged 15 commits into from
Mar 30, 2022
64 changes: 51 additions & 13 deletions .github/workflows/CI.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,17 @@ on:
branches:
- master
pull_request:
# schedule: TODO: unpin minor lib versions and check weekly
# - 0 0 0 ? * FRI *
paths-ignore:
- "*.md"
- "*.txt"
- ".asf.yaml"
- ".dlc.json"
- ".licenserc.yaml"
- "docs/*"
- ".github/workflows/*"
Copy link
Member

Choose a reason for hiding this comment

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

Pull requests require the CI check status context named CheckStatus to pass before they can be merged. And the check context is generated by the job CheckStatus in this file (line 123), if changes are in these paths-ignore, the workflow won't be run and the check status context won't be reported, thus all PRs can't be merged, you need to either remove these paths-ignore, or add a dummy workflow in a separate file that run a empty job named CheckStatus to generate the required context, but it is hard to set the reversed condition of these paths-ignore, you can see how I set in the main repo but that is still not perfect because under some cases the 2 workflows are all executed

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I see how it works, gonna fix it later.

Copy link
Member Author

Choose a reason for hiding this comment

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

Pull requests require the CI check status context named CheckStatus to pass before they can be merged. And the check context is generated by the job CheckStatus in this file (line 123), if changes are in these paths-ignore, the workflow won't be run and the check status context won't be reported, thus all PRs can't be merged, you need to either remove these paths-ignore, or add a dummy workflow in a separate file that run a empty job named CheckStatus to generate the required context, but it is hard to set the reversed condition of these paths-ignore, you can see how I set in the main repo but that is still not perfect because under some cases the 2 workflows are all executed

Looks like the paths_ignore option doesn't work like what I thought.. it's crazy.

Maybe the diff method is the ultimate solution given third party action cannot be run in asf repos.

Diff method:
https://github.community/t/paths-ignore-not-working-for-base-level-files/18445/4

Copy link
Member

Choose a reason for hiding this comment

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

Some third-party GHAs are approved by Apache infra and can be used, @Superskyyy take a look at this, though I haven't tried to use this GHA to achieve my goal as stated above

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm gonna try it and let you know if it works.

schedule: #TODO: unpin minor lib versions and check weekly with automation
- cron: '0 18 * * *'

concurrency:
group: ci-it-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true
Expand All @@ -34,7 +43,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Checkout source codes
uses: actions/checkout@v2
uses: actions/checkout@v3
with:
submodules: true
- name: Check License
Expand All @@ -45,47 +54,76 @@ jobs:
run: make lint
prep-plugin-and-unit-tests:
name: Prepare Plugin and Unit Tests
needs: license-and-lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v3
- id: set-matrix
run: |
sudo apt-get install jq
echo "::set-output name=matrix::$(bash tests/gather_test_paths.sh)"
outputs:
matrix: ${{ steps.set-matrix.outputs.matrix }}
docker:
name: Build Docker Image
runs-on: ubuntu-latest
strategy:
matrix: # may support pypy in the future
python-version: [ "3.6", "3.7", "3.8", "3.9", "3.10" ]
fail-fast: false
env:
BASE_PYTHON_VERSION: ${{ matrix.python-version }}
steps:
- name: Checkout source codes
uses: actions/checkout@v3
with:
submodules: true
- name: Build SkyWalking Python agent base plugin image
run: |
docker build --build-arg BASE_PYTHON_VERSION -t apache/skywalking-python-agent:latest-plugin --no-cache . -f tests/plugin/Dockerfile.plugin
docker save -o docker-images-skywalking-python-plugin-${{ matrix.python-version }}.tar apache/skywalking-python-agent:latest-plugin
- name: Upload docker image with specific python version
uses: actions/upload-artifact@v3
with:
Superskyyy marked this conversation as resolved.
Show resolved Hide resolved
name: docker-images-skywalking-python-plugin-${{ matrix.python-version }}
path: docker-images-skywalking-python-plugin-${{ matrix.python-version }}.tar

plugin-and-unit-tests:
name: Plugin and Unit Tests
needs: prep-plugin-and-unit-tests
needs: [ prep-plugin-and-unit-tests, docker ]
runs-on: ubuntu-latest
timeout-minutes: 30
strategy:
matrix:
python-version: ["3.6", "3.7", "3.8", "3.9", "3.10"]
python-version: [ "3.6", "3.7", "3.8", "3.9", "3.10" ]
test-path: ${{fromJson(needs.prep-plugin-and-unit-tests.outputs.matrix)}}
fail-fast: false
env:
SW_PYTHON_VERSION: ${{ matrix.python-version }}
BASE_PYTHON_VERSION: ${{ matrix.python-version }}
steps:
- name: Checkout source codes
uses: actions/checkout@v2
uses: actions/checkout@v3
with:
submodules: true
- name: Pull SkyWalking Python agent base image
uses: actions/download-artifact@v3
with:
name: docker-images-skywalking-python-plugin-${{ matrix.python-version }}
path: docker-images
- name: Load docker images
run: find docker-images -name "*.tar" -exec docker load -i {} \;
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v2
# This step is crucial for correct plugin matrix in test orchestration
uses: actions/setup-python@v3
with:
python-version: ${{ matrix.python-version }}
- name: Set up dependencies
run: make setup install
- name: Run unit tests
run: |
make test-parallel-setup
python3 -m pytest -v ${{ matrix.test-path }}
CheckStatus:
runs-on: ubuntu-latest
timeout-minutes: 60
needs: [plugin-and-unit-tests]
needs: [ license-and-lint, plugin-and-unit-tests ]
steps:
- name: Nothing
run: echo "Just to make the GitHub merge button green"
7 changes: 6 additions & 1 deletion .github/workflows/dead-link-checker.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ name: Dead Link Checker

on:
pull_request:
push:
branches:
- master
schedule:
- cron: '0 18 * * *'

concurrency:
group: dlc-${{ github.event.pull_request.number || github.ref }}
Expand All @@ -28,7 +33,7 @@ jobs:
runs-on: ubuntu-latest
timeout-minutes: 30
steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v3
- run: sudo npm install -g markdown-link-check
- run: |
for file in $(find . -name "*.md"); do
Expand Down
100 changes: 100 additions & 0 deletions .github/workflows/e2e.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
name: E2E

on:
pull_request:
paths-ignore:
- "*.md"
- "*.txt"
- ".asf.yaml"
- ".dlc.json"
- ".licenserc.yaml"
- "docs/*"
- ".github/workflows/*"
push:
branches:
- master
schedule:
- cron: '0 18 * * *'

concurrency:
group: ci-e2e-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true

jobs:
Docker:
name: Construct agent image
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- name: Checkout source codes
uses: actions/checkout@v3
with:
submodules: true
- name: Build SkyWalking Python agent base e2e image
run: |
docker build --build-arg BASE_PYTHON_VERSION -t apache/skywalking-python-agent:latest-e2e --no-cache . -f tests/e2e/base/Dockerfile.e2e
docker save -o docker-images-skywalking-python-e2e.tar apache/skywalking-python-agent:latest-e2e
env: # may support pypy in the future
BASE_PYTHON_VERSION: 3.7
- name: Upload docker image
uses: actions/upload-artifact@v3
with:
Superskyyy marked this conversation as resolved.
Show resolved Hide resolved
name: docker-images-skywalking-python-e2e
path: docker-images-skywalking-python-e2e.tar
E2E:
name: Basic function + ${{ matrix.case.name }} transport (Python3.7)
needs: Docker
runs-on: ubuntu-latest
timeout-minutes: 20
strategy:
matrix:
case:
- name: gRPC
path: tests/e2e/case/grpc/e2e.yaml
- name: HTTP
path: tests/e2e/case/http/e2e.yaml
- name: Kafka
path: tests/e2e/case/kafka/e2e.yaml
fail-fast: false
steps:
- name: Checkout source codes
uses: actions/checkout@v3
with:
submodules: true
- name: Pull SkyWalking Python agent base image
uses: actions/download-artifact@v3
with:
name: docker-images-skywalking-python-e2e
path: docker-images
- name: Load docker images
run: find docker-images -name "*.tar" -exec docker load -i {} \;
- name: Setup go
uses: actions/setup-go@v2
with:
go-version: '1.16'
- name: Run E2E Tests
uses: apache/skywalking-infra-e2e@main
with:
log-dir: /tmp/e2e-logs
e2e-file: ${{ matrix.case.path }}
- name: Upload Logs
uses: actions/upload-artifact@v3
if: ${{ failure() }}
with:
name: e2e_logs
path: "${{ env.SW_INFRA_E2E_LOG_DIR }}"
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ setup:
python3 -m pip install --upgrade pip
python3 -m pip install grpcio --ignore-installed

setup-test: setup
setup-test: setup gen
pip3 install -e .[test]

gen:
Expand Down
5 changes: 3 additions & 2 deletions docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.

ARG BASE_IMAGE
ARG BASE_PYTHON_VERSION
Superskyyy marked this conversation as resolved.
Show resolved Hide resolved

FROM ${BASE_IMAGE}
FROM ${BASE_PYTHON_VERSION}

ARG SW_PYTHON_AGENT_PROTOCOL
ARG SW_PYTHON_AGENT_VERSION

RUN pip install --no-cache-dir "apache-skywalking[${SW_PYTHON_AGENT_PROTOCOL}]==${SW_PYTHON_AGENT_VERSION}"

# So that the agent can be auto-started when container is started
ENTRYPOINT ["sw-python", "run"]
12 changes: 6 additions & 6 deletions docker/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ P := grpc http kafka

TARGETS := py3.6 py3.7 py3.8 py3.9 py3.10

py3.6: BASE_IMAGE = python:3.6
py3.7: BASE_IMAGE = python:3.7
py3.8: BASE_IMAGE = python:3.8
py3.9: BASE_IMAGE = python:3.9
py3.9: BASE_IMAGE = python:3.10
py3.6: BASE_PYTHON_VERSION = python:3.6
py3.7: BASE_PYTHON_VERSION = python:3.7
py3.8: BASE_PYTHON_VERSION = python:3.8
py3.9: BASE_PYTHON_VERSION = python:3.9
py3.9: BASE_PYTHON_VERSION = python:3.10

PUSH_TARGETS := $(TARGETS:%=push-%)

Expand All @@ -36,7 +36,7 @@ push: $(PUSH_TARGETS)
$(TARGETS):
for p in $(P); do \
$(D) build $(SW_BUILD_ARGS) \
--build-arg BASE_IMAGE=$(BASE_IMAGE) \
--build-arg BASE_PYTHON_VERSION=$(BASE_PYTHON_VERSION) \
--build-arg SW_PYTHON_AGENT_PROTOCOL=$$p \
--build-arg SW_PYTHON_AGENT_VERSION=${AGENT_VERSION} \
-t apache/skywalking-python:${AGENT_VERSION}-$$p-$@ \
Expand Down
2 changes: 1 addition & 1 deletion docs/en/setup/Container.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ it should be adopted and bootstrapped automatically through the `sw-python` CLI.
Provide the following arguments to build your own image from the dockerfile.

```text
BASE_IMAGE # the Python base image to build upon
BASE_PYTHON_VERSION # the Python base image to build upon
SW_PYTHON_AGENT_VERSION # agent version to be pulled from PyPI
SW_PYTHON_AGENT_PROTOCOL # agent protocol - grpc/ http/ kafka
```
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
'requests>=2.26.0',
],
'kafka': [
'kafka',
'kafka-python',
],
},
classifiers=[
Expand Down
2 changes: 1 addition & 1 deletion skywalking/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Component(Enum):
Celery = 7011
Falcon = 7012
MysqlClient = 7013
FastApi = 7014
FastAPI = 7014


class Layer(Enum):
Expand Down
3 changes: 1 addition & 2 deletions skywalking/agent/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@
from threading import Thread, Event
from typing import TYPE_CHECKING

from skywalking.protocol.logging.Logging_pb2 import LogData

from skywalking import config, plugins
from skywalking import loggings
from skywalking import profile
Expand All @@ -31,6 +29,7 @@
from skywalking.loggings import logger
from skywalking.profile.profile_task import ProfileTask
from skywalking.profile.snapshot import TracingThreadSnapshot
from skywalking.protocol.logging.Logging_pb2 import LogData

if TYPE_CHECKING:
from skywalking.trace.context import Segment
Expand Down
1 change: 1 addition & 0 deletions skywalking/client/grpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def __init__(self, channel: grpc.Channel):
self.service_stub = ManagementServiceStub(channel)

def send_instance_props(self):
# TODO: other properties periodically | matching behavior of java agent
self.service_stub.reportInstanceProperties(InstanceProperties(
service=config.service_name,
serviceInstance=config.service_instance,
Expand Down
2 changes: 1 addition & 1 deletion skywalking/client/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def send_instance_props(self):
'service': config.service_name,
'serviceInstance': config.service_instance,
'properties': [
{'key': 'language', 'value': 'Python'},
{'key': 'language', 'value': 'python'},
]
})
if logger_debug_enabled:
Expand Down
2 changes: 1 addition & 1 deletion skywalking/plugins/sw_fastapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ async def _sw_fast_api(self, scope: Scope, receive: Receive, send: Send):

with span:
span.layer = Layer.Http
span.component = Component.FastApi
span.component = Component.FastAPI
span.peer = f'{req.client.host}:{req.client.port}'
span.tag(TagHttpMethod(method))
span.tag(TagHttpURL(req.url._url.split('?')[0]))
Expand Down
35 changes: 35 additions & 0 deletions tests/e2e/base/Dockerfile.e2e
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# Builds -> skywalking-agent:latest-e2e
ARG BASE_PYTHON_VERSION

FROM python:${BASE_PYTHON_VERSION}

VOLUME /services

COPY tests/e2e/base/consumer/consumer.py /services/
COPY tests/e2e/base/provider/provider.py /services/

# Copy the project and build
COPY . /skywalking-python/
RUN cd /skywalking-python && make install

RUN pip install requests kafka-python
# Extra dependencies for e2e services
RUN pip install fastapi uvicorn aiohttp

# Entrypoint with agent attached
Entrypoint ["sw-python", "run"]