-
Notifications
You must be signed in to change notification settings - Fork 2
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
Tempo tracing #20
base: main
Are you sure you want to change the base?
Tempo tracing #20
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
license-eye has checked 264 files.
Valid | Invalid | Ignored | Fixed |
---|---|---|---|
0 | 148 | 116 | 0 |
Click to see the invalid file list
- examples/django/charm/charmcraft.yaml
- examples/django/charm/src/charm.py
- examples/django/django_app/django_app/django_app/init.py
- examples/django/django_app/django_app/django_app/asgi.py
- examples/django/django_app/django_app/django_app/settings.py
- examples/django/django_app/django_app/django_app/urls.py
- examples/django/django_app/django_app/django_app/wsgi.py
- examples/django/django_app/django_app/manage.py
- examples/django/django_app/django_app/migrate.sh
- examples/django/django_app/django_app/testing/init.py
- examples/django/django_app/django_app/testing/admin.py
- examples/django/django_app/django_app/testing/apps.py
- examples/django/django_app/django_app/testing/migrations/init.py
- examples/django/django_app/django_app/testing/models.py
- examples/django/django_app/django_app/testing/tests.py
- examples/django/django_app/django_app/testing/views.py
- examples/django/django_app/rockcraft.yaml
- examples/django/django_async_app/django_async_app/django_async_app/init.py
- examples/django/django_async_app/django_async_app/django_async_app/asgi.py
- examples/django/django_async_app/django_async_app/django_async_app/settings.py
- examples/django/django_async_app/django_async_app/django_async_app/urls.py
- examples/django/django_async_app/django_async_app/django_async_app/wsgi.py
- examples/django/django_async_app/django_async_app/manage.py
- examples/django/django_async_app/django_async_app/testing/init.py
- examples/django/django_async_app/django_async_app/testing/admin.py
- examples/django/django_async_app/django_async_app/testing/apps.py
- examples/django/django_async_app/django_async_app/testing/migrations/init.py
- examples/django/django_async_app/django_async_app/testing/models.py
- examples/django/django_async_app/django_async_app/testing/tests.py
- examples/django/django_async_app/django_async_app/testing/views.py
- examples/django/django_async_app/rockcraft.yaml
- examples/fastapi/alembic.ini
- examples/fastapi/alembic/env.py
- examples/fastapi/alembic/versions/eca6177bd16a_initial_migration.py
- examples/fastapi/app.py
- examples/fastapi/charm/charmcraft.yaml
- examples/fastapi/charm/src/charm.py
- examples/fastapi/migrate.sh
- examples/fastapi/rockcraft.yaml
- examples/flask/charmcraft.yaml
- examples/flask/src/charm.py
- examples/flask/test_async_rock/app.py
- examples/flask/test_async_rock/rockcraft.yaml
- examples/flask/test_db_rock/alembic.ini
- examples/flask/test_db_rock/alembic/env.py
- examples/flask/test_db_rock/alembic/versions/eca6177bd16a_initial_migration.py
- examples/flask/test_db_rock/app.py
- examples/flask/test_db_rock/migrate.sh
- examples/flask/test_db_rock/rockcraft.yaml
- examples/flask/test_rock/app.py
- examples/flask/test_rock/rockcraft.yaml
- examples/go/charm/charmcraft.yaml
- examples/go/charm/src/charm.py
- examples/go/go.mod
- examples/go/internal/service/service.go
- examples/go/main.go
- examples/go/migrate.sh
- examples/go/rockcraft.yaml
- localstack-installation.sh
- pyproject.toml
- setup.py
- src/paas_app_charmer/init.py
- src/paas_app_charmer/django/init.py
- src/paas_app_charmer/django/charm.py
- src/paas_app_charmer/fastapi/init.py
- src/paas_app_charmer/fastapi/charm.py
- src/paas_app_charmer/flask/init.py
- src/paas_app_charmer/flask/charm.py
- src/paas_app_charmer/go/init.py
- src/paas_app_charmer/go/charm.py
- src/paas_charm/init.py
- src/paas_charm/_gunicorn/init.py
- src/paas_charm/_gunicorn/charm.py
- src/paas_charm/_gunicorn/webserver.py
- src/paas_charm/_gunicorn/workload_config.py
- src/paas_charm/_gunicorn/wsgi_app.py
- src/paas_charm/app.py
- src/paas_charm/charm.py
- src/paas_charm/charm_state.py
- src/paas_charm/charm_utils.py
- src/paas_charm/database_migration.py
- src/paas_charm/databases.py
- src/paas_charm/django/init.py
- src/paas_charm/django/charm.py
- src/paas_charm/exceptions.py
- src/paas_charm/fastapi/init.py
- src/paas_charm/fastapi/charm.py
- src/paas_charm/flask/init.py
- src/paas_charm/flask/charm.py
- src/paas_charm/framework.py
- src/paas_charm/go/init.py
- src/paas_charm/go/charm.py
- src/paas_charm/observability.py
- src/paas_charm/rabbitmq.py
- src/paas_charm/secret_storage.py
- src/paas_charm/utils.py
- tests/init.py
- tests/conftest.py
- tests/integration/conftest.py
- tests/integration/django/init.py
- tests/integration/django/conftest.py
- tests/integration/django/test_django.py
- tests/integration/django/test_django_integrations.py
- tests/integration/django/test_workers.py
- tests/integration/fastapi/init.py
- tests/integration/fastapi/conftest.py
- tests/integration/fastapi/test_fastapi.py
- tests/integration/flask/init.py
- tests/integration/flask/conftest.py
- tests/integration/flask/test_charm.py
- tests/integration/flask/test_cos.py
- tests/integration/flask/test_database.py
- tests/integration/flask/test_db_migration.py
- tests/integration/flask/test_integrations.py
- tests/integration/flask/test_proxy.py
- tests/integration/flask/test_workers.py
- tests/integration/go/init.py
- tests/integration/go/conftest.py
- tests/integration/go/test_go.py
- tests/integration/helpers.py
- tests/unit/init.py
- tests/unit/django/init.py
- tests/unit/django/conftest.py
- tests/unit/django/constants.py
- tests/unit/django/test_charm.py
- tests/unit/django/test_workers.py
- tests/unit/fastapi/init.py
- tests/unit/fastapi/conftest.py
- tests/unit/fastapi/constants.py
- tests/unit/fastapi/test_charm.py
- tests/unit/flask/init.py
- tests/unit/flask/conftest.py
- tests/unit/flask/constants.py
- tests/unit/flask/test_charm.py
- tests/unit/flask/test_charm_state.py
- tests/unit/flask/test_database_migration.py
- tests/unit/flask/test_databases.py
- tests/unit/flask/test_flask_app.py
- tests/unit/flask/test_tracing.py
- tests/unit/flask/test_webserver.py
- tests/unit/flask/test_workers.py
- tests/unit/go/init.py
- tests/unit/go/conftest.py
- tests/unit/go/constants.py
- tests/unit/go/test_app.py
- tests/unit/go/test_charm.py
- tests/unit/test_deprecated.py
- tox.ini
Use this command to fix any missing license headers
```bash
docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix
</details>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
license-eye has checked 264 files.
Valid | Invalid | Ignored | Fixed |
---|---|---|---|
0 | 148 | 116 | 0 |
Click to see the invalid file list
- examples/django/charm/charmcraft.yaml
- examples/django/charm/src/charm.py
- examples/django/django_app/django_app/django_app/init.py
- examples/django/django_app/django_app/django_app/asgi.py
- examples/django/django_app/django_app/django_app/settings.py
- examples/django/django_app/django_app/django_app/urls.py
- examples/django/django_app/django_app/django_app/wsgi.py
- examples/django/django_app/django_app/manage.py
- examples/django/django_app/django_app/migrate.sh
- examples/django/django_app/django_app/testing/init.py
- examples/django/django_app/django_app/testing/admin.py
- examples/django/django_app/django_app/testing/apps.py
- examples/django/django_app/django_app/testing/migrations/init.py
- examples/django/django_app/django_app/testing/models.py
- examples/django/django_app/django_app/testing/tests.py
- examples/django/django_app/django_app/testing/views.py
- examples/django/django_app/rockcraft.yaml
- examples/django/django_async_app/django_async_app/django_async_app/init.py
- examples/django/django_async_app/django_async_app/django_async_app/asgi.py
- examples/django/django_async_app/django_async_app/django_async_app/settings.py
- examples/django/django_async_app/django_async_app/django_async_app/urls.py
- examples/django/django_async_app/django_async_app/django_async_app/wsgi.py
- examples/django/django_async_app/django_async_app/manage.py
- examples/django/django_async_app/django_async_app/testing/init.py
- examples/django/django_async_app/django_async_app/testing/admin.py
- examples/django/django_async_app/django_async_app/testing/apps.py
- examples/django/django_async_app/django_async_app/testing/migrations/init.py
- examples/django/django_async_app/django_async_app/testing/models.py
- examples/django/django_async_app/django_async_app/testing/tests.py
- examples/django/django_async_app/django_async_app/testing/views.py
- examples/django/django_async_app/rockcraft.yaml
- examples/fastapi/alembic.ini
- examples/fastapi/alembic/env.py
- examples/fastapi/alembic/versions/eca6177bd16a_initial_migration.py
- examples/fastapi/app.py
- examples/fastapi/charm/charmcraft.yaml
- examples/fastapi/charm/src/charm.py
- examples/fastapi/migrate.sh
- examples/fastapi/rockcraft.yaml
- examples/flask/charmcraft.yaml
- examples/flask/src/charm.py
- examples/flask/test_async_rock/app.py
- examples/flask/test_async_rock/rockcraft.yaml
- examples/flask/test_db_rock/alembic.ini
- examples/flask/test_db_rock/alembic/env.py
- examples/flask/test_db_rock/alembic/versions/eca6177bd16a_initial_migration.py
- examples/flask/test_db_rock/app.py
- examples/flask/test_db_rock/migrate.sh
- examples/flask/test_db_rock/rockcraft.yaml
- examples/flask/test_rock/app.py
- examples/flask/test_rock/rockcraft.yaml
- examples/go/charm/charmcraft.yaml
- examples/go/charm/src/charm.py
- examples/go/go.mod
- examples/go/internal/service/service.go
- examples/go/main.go
- examples/go/migrate.sh
- examples/go/rockcraft.yaml
- localstack-installation.sh
- pyproject.toml
- setup.py
- src/paas_app_charmer/init.py
- src/paas_app_charmer/django/init.py
- src/paas_app_charmer/django/charm.py
- src/paas_app_charmer/fastapi/init.py
- src/paas_app_charmer/fastapi/charm.py
- src/paas_app_charmer/flask/init.py
- src/paas_app_charmer/flask/charm.py
- src/paas_app_charmer/go/init.py
- src/paas_app_charmer/go/charm.py
- src/paas_charm/init.py
- src/paas_charm/_gunicorn/init.py
- src/paas_charm/_gunicorn/charm.py
- src/paas_charm/_gunicorn/webserver.py
- src/paas_charm/_gunicorn/workload_config.py
- src/paas_charm/_gunicorn/wsgi_app.py
- src/paas_charm/app.py
- src/paas_charm/charm.py
- src/paas_charm/charm_state.py
- src/paas_charm/charm_utils.py
- src/paas_charm/database_migration.py
- src/paas_charm/databases.py
- src/paas_charm/django/init.py
- src/paas_charm/django/charm.py
- src/paas_charm/exceptions.py
- src/paas_charm/fastapi/init.py
- src/paas_charm/fastapi/charm.py
- src/paas_charm/flask/init.py
- src/paas_charm/flask/charm.py
- src/paas_charm/framework.py
- src/paas_charm/go/init.py
- src/paas_charm/go/charm.py
- src/paas_charm/observability.py
- src/paas_charm/rabbitmq.py
- src/paas_charm/secret_storage.py
- src/paas_charm/utils.py
- tests/init.py
- tests/conftest.py
- tests/integration/conftest.py
- tests/integration/django/init.py
- tests/integration/django/conftest.py
- tests/integration/django/test_django.py
- tests/integration/django/test_django_integrations.py
- tests/integration/django/test_workers.py
- tests/integration/fastapi/init.py
- tests/integration/fastapi/conftest.py
- tests/integration/fastapi/test_fastapi.py
- tests/integration/flask/init.py
- tests/integration/flask/conftest.py
- tests/integration/flask/test_charm.py
- tests/integration/flask/test_cos.py
- tests/integration/flask/test_database.py
- tests/integration/flask/test_db_migration.py
- tests/integration/flask/test_integrations.py
- tests/integration/flask/test_proxy.py
- tests/integration/flask/test_workers.py
- tests/integration/go/init.py
- tests/integration/go/conftest.py
- tests/integration/go/test_go.py
- tests/integration/helpers.py
- tests/unit/init.py
- tests/unit/django/init.py
- tests/unit/django/conftest.py
- tests/unit/django/constants.py
- tests/unit/django/test_charm.py
- tests/unit/django/test_workers.py
- tests/unit/fastapi/init.py
- tests/unit/fastapi/conftest.py
- tests/unit/fastapi/constants.py
- tests/unit/fastapi/test_charm.py
- tests/unit/flask/init.py
- tests/unit/flask/conftest.py
- tests/unit/flask/constants.py
- tests/unit/flask/test_charm.py
- tests/unit/flask/test_charm_state.py
- tests/unit/flask/test_database_migration.py
- tests/unit/flask/test_databases.py
- tests/unit/flask/test_flask_app.py
- tests/unit/flask/test_tracing.py
- tests/unit/flask/test_webserver.py
- tests/unit/flask/test_workers.py
- tests/unit/go/init.py
- tests/unit/go/conftest.py
- tests/unit/go/constants.py
- tests/unit/go/test_app.py
- tests/unit/go/test_charm.py
- tests/unit/test_deprecated.py
- tox.ini
Use this command to fix any missing license headers
```bash
docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix
</details>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
license-eye has checked 300 files.
Valid | Invalid | Ignored | Fixed |
---|---|---|---|
0 | 177 | 123 | 0 |
Click to see the invalid file list
- app.py
- examples/django/charm/charmcraft.yaml
- examples/django/charm/src/charm.py
- examples/django/django_app/django_app/django_app/init.py
- examples/django/django_app/django_app/django_app/asgi.py
- examples/django/django_app/django_app/django_app/settings.py
- examples/django/django_app/django_app/django_app/urls.py
- examples/django/django_app/django_app/django_app/wsgi.py
- examples/django/django_app/django_app/manage.py
- examples/django/django_app/django_app/migrate.sh
- examples/django/django_app/django_app/testing/init.py
- examples/django/django_app/django_app/testing/admin.py
- examples/django/django_app/django_app/testing/apps.py
- examples/django/django_app/django_app/testing/migrations/init.py
- examples/django/django_app/django_app/testing/models.py
- examples/django/django_app/django_app/testing/tests.py
- examples/django/django_app/django_app/testing/views.py
- examples/django/django_app/rockcraft.yaml
- examples/django/django_async_app/django_async_app/django_async_app/init.py
- examples/django/django_async_app/django_async_app/django_async_app/asgi.py
- examples/django/django_async_app/django_async_app/django_async_app/settings.py
- examples/django/django_async_app/django_async_app/django_async_app/urls.py
- examples/django/django_async_app/django_async_app/django_async_app/wsgi.py
- examples/django/django_async_app/django_async_app/manage.py
- examples/django/django_async_app/django_async_app/testing/init.py
- examples/django/django_async_app/django_async_app/testing/admin.py
- examples/django/django_async_app/django_async_app/testing/apps.py
- examples/django/django_async_app/django_async_app/testing/migrations/init.py
- examples/django/django_async_app/django_async_app/testing/models.py
- examples/django/django_async_app/django_async_app/testing/tests.py
- examples/django/django_async_app/django_async_app/testing/views.py
- examples/django/django_async_app/rockcraft.yaml
- examples/django/django_tracing_app/django_tracing_app/django_tracing_app/init.py
- examples/django/django_tracing_app/django_tracing_app/django_tracing_app/asgi.py
- examples/django/django_tracing_app/django_tracing_app/django_tracing_app/settings.py
- examples/django/django_tracing_app/django_tracing_app/django_tracing_app/urls.py
- examples/django/django_tracing_app/django_tracing_app/django_tracing_app/wsgi.py
- examples/django/django_tracing_app/django_tracing_app/manage.py
- examples/django/django_tracing_app/django_tracing_app/migrate.sh
- examples/django/django_tracing_app/django_tracing_app/testing/init.py
- examples/django/django_tracing_app/django_tracing_app/testing/admin.py
- examples/django/django_tracing_app/django_tracing_app/testing/apps.py
- examples/django/django_tracing_app/django_tracing_app/testing/migrations/init.py
- examples/django/django_tracing_app/django_tracing_app/testing/models.py
- examples/django/django_tracing_app/django_tracing_app/testing/tests.py
- examples/django/django_tracing_app/django_tracing_app/testing/views.py
- examples/django/django_tracing_app/rockcraft.yaml
- examples/fastapi/charm/charmcraft.yaml
- examples/fastapi/charm/src/charm.py
- examples/fastapi/fastapi_app/alembic.ini
- examples/fastapi/fastapi_app/alembic/env.py
- examples/fastapi/fastapi_app/alembic/versions/eca6177bd16a_initial_migration.py
- examples/fastapi/fastapi_app/app.py
- examples/fastapi/fastapi_app/migrate.sh
- examples/fastapi/fastapi_app/rockcraft.yaml
- examples/fastapi/fastapi_tracing_app/alembic.ini
- examples/fastapi/fastapi_tracing_app/alembic/env.py
- examples/fastapi/fastapi_tracing_app/alembic/versions/eca6177bd16a_initial_migration.py
- examples/fastapi/fastapi_tracing_app/app.py
- examples/fastapi/fastapi_tracing_app/migrate.sh
- examples/fastapi/fastapi_tracing_app/rockcraft.yaml
- examples/flask/charmcraft.yaml
- examples/flask/src/charm.py
- examples/flask/test_async_rock/app.py
- examples/flask/test_async_rock/rockcraft.yaml
- examples/flask/test_db_rock/alembic.ini
- examples/flask/test_db_rock/alembic/env.py
- examples/flask/test_db_rock/alembic/versions/eca6177bd16a_initial_migration.py
- examples/flask/test_db_rock/app.py
- examples/flask/test_db_rock/migrate.sh
- examples/flask/test_db_rock/rockcraft.yaml
- examples/flask/test_rock/app.py
- examples/flask/test_rock/rockcraft.yaml
- examples/flask/test_tracing_rock/app.py
- examples/flask/test_tracing_rock/rockcraft.yaml
- examples/go/charm/charmcraft.yaml
- examples/go/charm/src/charm.py
- examples/go/go_app/go.mod
- examples/go/go_app/internal/service/service.go
- examples/go/go_app/main.go
- examples/go/go_app/migrate.sh
- examples/go/go_app/rockcraft.yaml
- examples/go/go_tracing_app/go.mod
- examples/go/go_tracing_app/internal/service/service.go
- examples/go/go_tracing_app/main.go
- examples/go/go_tracing_app/migrate.sh
- examples/go/go_tracing_app/rockcraft.yaml
- localstack-installation.sh
- pyproject.toml
- setup.py
- src/paas_app_charmer/init.py
- src/paas_app_charmer/django/init.py
- src/paas_app_charmer/django/charm.py
- src/paas_app_charmer/fastapi/init.py
- src/paas_app_charmer/fastapi/charm.py
- src/paas_app_charmer/flask/init.py
- src/paas_app_charmer/flask/charm.py
- src/paas_app_charmer/go/init.py
- src/paas_app_charmer/go/charm.py
- src/paas_charm/init.py
- src/paas_charm/_gunicorn/init.py
- src/paas_charm/_gunicorn/charm.py
- src/paas_charm/_gunicorn/webserver.py
- src/paas_charm/_gunicorn/workload_config.py
- src/paas_charm/_gunicorn/wsgi_app.py
- src/paas_charm/app.py
- src/paas_charm/charm.py
- src/paas_charm/charm_state.py
- src/paas_charm/charm_utils.py
- src/paas_charm/database_migration.py
- src/paas_charm/databases.py
- src/paas_charm/django/init.py
- src/paas_charm/django/charm.py
- src/paas_charm/exceptions.py
- src/paas_charm/fastapi/init.py
- src/paas_charm/fastapi/charm.py
- src/paas_charm/flask/init.py
- src/paas_charm/flask/charm.py
- src/paas_charm/framework.py
- src/paas_charm/go/init.py
- src/paas_charm/go/charm.py
- src/paas_charm/observability.py
- src/paas_charm/rabbitmq.py
- src/paas_charm/secret_storage.py
- src/paas_charm/utils.py
- tests/init.py
- tests/conftest.py
- tests/integration/conftest.py
- tests/integration/django/init.py
- tests/integration/django/conftest.py
- tests/integration/django/test_django.py
- tests/integration/django/test_django_integrations.py
- tests/integration/django/test_workers.py
- tests/integration/fastapi/init.py
- tests/integration/fastapi/conftest.py
- tests/integration/fastapi/test_fastapi.py
- tests/integration/flask/init.py
- tests/integration/flask/conftest.py
- tests/integration/flask/test_charm.py
- tests/integration/flask/test_cos.py
- tests/integration/flask/test_database.py
- tests/integration/flask/test_db_migration.py
- tests/integration/flask/test_integrations.py
- tests/integration/flask/test_proxy.py
- tests/integration/flask/test_workers.py
- tests/integration/go/init.py
- tests/integration/go/conftest.py
- tests/integration/go/test_go.py
- tests/integration/helpers.py
- tests/unit/init.py
- tests/unit/django/init.py
- tests/unit/django/conftest.py
- tests/unit/django/constants.py
- tests/unit/django/test_charm.py
- tests/unit/django/test_workers.py
- tests/unit/fastapi/init.py
- tests/unit/fastapi/conftest.py
- tests/unit/fastapi/constants.py
- tests/unit/fastapi/test_charm.py
- tests/unit/flask/init.py
- tests/unit/flask/conftest.py
- tests/unit/flask/constants.py
- tests/unit/flask/test_charm.py
- tests/unit/flask/test_charm_state.py
- tests/unit/flask/test_database_migration.py
- tests/unit/flask/test_databases.py
- tests/unit/flask/test_flask_app.py
- tests/unit/flask/test_tracing.py
- tests/unit/flask/test_webserver.py
- tests/unit/flask/test_workers.py
- tests/unit/go/init.py
- tests/unit/go/conftest.py
- tests/unit/go/constants.py
- tests/unit/go/test_app.py
- tests/unit/go/test_charm.py
- tests/unit/test_deprecated.py
- tox.ini
Use this command to fix any missing license headers
```bash
docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix
</details>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@pytest.mark.parametrize( | ||
"tracing_app, port", | ||
[ | ||
("flask_tracing_app", 8000), | ||
("django_tracing_app", 8000), | ||
("fastapi_tracing_app", 8080), | ||
("go_tracing_app", 8080), | ||
], | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can use indirect
fixture by using indirect=["tracing_app"]
This will avoid calling getfixturevalue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to use this but it turns out it doesn't call the flask_tracing_app
when you use indirect
. What it does is calls the tracing_app
fixture with a parameter that is flask_tracing_app
. If I do that I have to create a fixture like the following:
@pytest.fixture(name="tracing_app")
async def tracing_app_fixture(request):
return await request.getfixturevalue(request.param)
I think this is worse then just using the getfixturevalue
inside the test. Since in both cases we use that function and in indirect
method we add more lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if you think indirect
method is better I can implement it.
tracing_app = request.getfixturevalue(tracing_app) | ||
idle_list = [tracing_app.name] | ||
|
||
if tracing_app != "flask_tracing_app": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably be tracing_app.name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed re-assigning the variable issues. Thanks for pointing out
from .constants import DEFAULT_LAYER, FLASK_CONTAINER_NAME | ||
|
||
|
||
def test_tracing_relation(harness: Harness): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add a test with tracing disabled and ensure the envvars aren't injected
And I think you can also add a test in test_webserver.py that ensure the generated gunicorn config contains the tracing methods
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will also try to add a test for the following lines:
try:
# pylint: disable=ungrouped-imports
from charms.tempo_coordinator_k8s.v0.tracing import TracingEndpointRequirer
except ImportError:
logger.exception(
"Missing charm library, please run "
"`charmcraft fetch-lib charms.tempo_coordinator_k8s.v0.tracing`"
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work!
Just a question (maybe I it is already discussed). As the examples are similar to the base ones, why not updating the base examples instead of adding new ones?
"OTEL_EXPORTER_OTLP_ENDPOINT", None | ||
): | ||
config += textwrap.dedent( | ||
"""\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it could be simpler to put the configuration in a jinja template or similar, as it it slowly getting complex
@@ -20,6 +20,95 @@ | |||
logger = logging.getLogger(__name__) | |||
|
|||
|
|||
class TempoParameters(BaseModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although ordering the classes so everything is defined before is used has advantages, it also has disadvantages.
The disadvantage of putting this class as the first one of the file, is that when opening it, it takes a while to realise that the most important class is CharmState and what it contains. In this way readibility is decreased. This is similar to https://github.com/canonical/is-charms-contributing-guide/blob/main/CONTRIBUTING.md#function-and-method-Ordering
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT? @jdkandersson
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason is around that these are classes that other classes depend on so you have to define them first I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our case the difference is this:
- s3_parameters: "S3Parameters | None" = None
- saml_parameters: "SamlParameters | None" = None
+ s3_parameters: S3Parameters | None = None
+ saml_parameters: SamlParameters | None = None
the type hints have to be defined with strings. It is of course a matter of preference. My opinion leans toward using strings (that are checked by the linter), although they are not as elegant.
|
||
PROJECT_ROOT = pathlib.Path(__file__).parent.parent.parent.parent | ||
|
||
import nest_asyncio |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this needed?
""" | ||
|
||
try: | ||
tempo_app = await request.getfixturevalue("tempo_app") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not having a fixture for this?
|
||
if tracing_app.name != "flask-tracing-k8s": | ||
try: | ||
postgresql_app = request.getfixturevalue("postgresql_k8s") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could postgresq_k8s be integrated in the django_tracing_app /fastapi_tracing_app instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would copy the same piece of code to 3 different places(go, fastapi, django). WDYT @jdkandersson ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There will be two more lines in each those three fixtures. However in this if
there are already 8 lines. Besides, all the fixtures will be complete and equal, and also could be reused in other tests (currently, for example the flask-tracing-k8s
waits for the app to be active but not the others).
|
||
import ops | ||
import pytest | ||
from ops.testing import Harness |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it be very difficult to write the new tests using scenario as harness is legacy
?
Test coverage for 4f05ded
Static code analysis report
|
I think I agree with Javi that we should just use existing examples and add tempo to them. Adding a lot of extra examples is a lot to maintain |
Overview
Adds new integration Tempo to the charm. Creates a class called
TempoParameters
that handles the parameters. There are only 2 parameters:Endpoint comes from the integration and service name is generated by reading the charm name from the
charmcraft.yaml
file.Rationale
Juju Events Changes
Module Changes
Library Changes
Checklist
src-docs
urgent
,trivial
,complex
)