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

Convert IIB_OTEL_TRACING to an env variable #527

Merged
merged 1 commit into from
Jun 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,6 @@ The custom configuration options for the REST API are listed below:
* `IIB_LOG_LEVEL` - the Python log level of the REST API (Flask). This defaults to `INFO`.
* `IIB_MAX_PER_PAGE` - the maximum number of build requests that can be shown on a single page.
This defaults to `20`.
* `IIB_OTEL_TRACING` - the boolean value which indicates if OpenTelemetry tracing is enabled in IIB.
This defaults to `False`.
* `IIB_REQUEST_DATA_DAYS_TO_LIVE` - the amount of days after which per request temmporary data is
considered to be expired and may be removed. This defaults to `3`.
* `IIB_REQUEST_LOGS_DIR` - the directory to load the request specific log files. If `None`, per
Expand Down Expand Up @@ -271,8 +269,12 @@ More info on these environment variables can be found in the [AWS User Guide](ht

### Opentelemetry Environment Variable

To integrate IIB with Opentelemetery tracing, we will need the below two parameters.
To integrate IIB with Opentelemetery tracing, we will need the following parameters as
environment variables.

* `IIB_OTEL_TRACING` - the boolean value which indicates if OpenTelemetry tracing is enabled in IIB.
This defaults to `False`. If this is set to `True`, `OTEL_EXPORTER_OTLP_ENDPOINT` and `OTEL_SERVICE_NAME`
must be set.
* `OTEL_EXPORTER_OTLP_ENDPOINT` - The endpoint for the OpenTelemetry exporter.
* `OTEL_SERVICE_NAME` - "iib-api"

Expand Down Expand Up @@ -397,8 +399,6 @@ The custom configuration options for the Celery workers are listed below:
}
```

* `iib_otel_tracing` - the boolean value which indicates if OpenTelemetry tracing is enabled in IIB.
This defaults to `False`.
* `iib_request_related_bundles_dir` - the directory to write the request specific related bundles
file. If `None`, per request related bundles files are not created. This defaults to `None`.
* `iib_request_logs_dir` - the directory to write the request specific log files. If `None`, per
Expand Down Expand Up @@ -433,8 +433,12 @@ More info on these environment variables can be found in the [AWS User Guide](ht

### Opentelemetry Environment Variable

To integrate IIB with Opentelemetery tracing, we will need the below two parameters.
To integrate IIB with Opentelemetery tracing, we will need the following parameters
as environment variables.

* `IIB_OTEL_TRACING` - the boolean value which indicates if OpenTelemetry tracing is enabled in IIB.
This defaults to `False`. If this is set to `True`, `OTEL_EXPORTER_OTLP_ENDPOINT` and `OTEL_SERVICE_NAME`
must be set.
* `OTEL_EXPORTER_OTLP_ENDPOINT` - The endpoint for the OpenTelemetry exporter.
* `OTEL_SERVICE_NAME` - "iib-workers"

Expand Down
5 changes: 2 additions & 3 deletions iib/common/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ def func():
from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter
from opentelemetry.trace.propagation.tracecontext import TraceContextTextMapPropagator

from iib.web.config import Config

log = logging.getLogger(__name__)
propagator = TraceContextTextMapPropagator()
Expand All @@ -43,7 +42,7 @@ class TracingWrapper:

def __new__(cls):
"""Create a new instance if one does not exist."""
if not Config.IIB_OTEL_TRACING:
if not os.getenv('IIB_OTEL_TRACING', '').lower() == 'true':
return None

if TracingWrapper.__instance is None:
Expand Down Expand Up @@ -87,7 +86,7 @@ def instrument_tracing(
def decorator_instrument_tracing(func):
@functools.wraps(func)
def wrapper(*args, **kwargs):
if not Config.IIB_OTEL_TRACING:
if not os.getenv('IIB_OTEL_TRACING', '').lower() == 'true':
return func(*args, **kwargs)
log.debug('Context inside %s: %s', span_name, context)
if kwargs.get('traceparent'):
Expand Down
6 changes: 2 additions & 4 deletions iib/web/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,7 @@ def validate_api_config(config: Config) -> None:
'These are used for read/write access to the s3 bucket by IIB'
)

if config['IIB_OTEL_TRACING']:
if not isinstance(config['IIB_OTEL_TRACING'], bool):
raise ConfigError('"IIB_OTEL_TRACING" must be a valid boolean value')
if os.getenv('IIB_OTEL_TRACING', '').lower() == 'true':
if not os.getenv('OTEL_EXPORTER_OTLP_ENDPOINT') or not os.getenv('OTEL_SERVICE_NAME'):
raise ConfigError(
'"OTEL_EXPORTER_OTLP_ENDPOINT" and "OTEL_SERVICE_NAME" environment '
Expand Down Expand Up @@ -241,7 +239,7 @@ def create_app(config_obj: Optional[str] = None) -> Flask: # pragma: no cover
app.register_error_handler(kombu.exceptions.KombuError, json_error)

# Add Auto-instrumentation
if app.config['IIB_OTEL_TRACING']:
if os.getenv('IIB_OTEL_TRACING', '').lower() == 'true':
FlaskInstrumentor().instrument_app(
app, enable_commenter=True, commenter_options={}, tracer_provider=tracerWrapper.provider
)
Expand Down
1 change: 0 additions & 1 deletion iib/web/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ class Config(object):
IIB_MESSAGING_DURABLE: bool = True
IIB_MESSAGING_KEY: str = '/etc/iib/messaging.key'
IIB_MESSAGING_TIMEOUT: int = 30
IIB_OTEL_TRACING: bool = False
IIB_REQUEST_DATA_DAYS_TO_LIVE: int = 3
IIB_REQUEST_LOGS_DIR: Optional[str] = None
IIB_REQUEST_RELATED_BUNDLES_DIR: Optional[str] = None
Expand Down
7 changes: 2 additions & 5 deletions iib/workers/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ class Config(object):
iib_log_level: str = 'INFO'
iib_max_recursive_related_bundles = 15
iib_organization_customizations: iib_organization_customizations_type = {}
iib_otel_tracing: bool = False
iib_sac_queues: List[str] = []
iib_request_logs_dir: Optional[str] = None
iib_request_logs_format: str = (
Expand Down Expand Up @@ -320,13 +319,11 @@ def validate_celery_config(conf: app.utils.Settings, **kwargs) -> None:
if not os.access(iib_request_temp_data_dir, os.W_OK):
raise ConfigError(f'{directory}, is not writable!')

if conf.get('iib_otel_tracing'):
if not isinstance(conf.get('iib_otel_tracing'), bool):
raise ConfigError('"iib_otel_tracing" must be a valid boolean value')
if os.getenv('IIB_OTEL_TRACING', '').lower() == 'true':
if not os.getenv('OTEL_EXPORTER_OTLP_ENDPOINT') or not os.getenv('OTEL_SERVICE_NAME'):
raise ConfigError(
'"OTEL_EXPORTER_OTLP_ENDPOINT" and "OTEL_SERVICE_NAME" environment '
'variables must be set to valid strings when iib_otel_tracing is set to True.'
'variables must be set to valid strings when "IIB_OTEL_TRACING" is set to True.'
)


Expand Down
6 changes: 4 additions & 2 deletions iib/workers/tasks/celery.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# SPDX-License-Identifier: GPL-3.0-or-later
import os

import celery
from celery.signals import celeryd_init
from opentelemetry.instrumentation.celery import CeleryInstrumentor
Expand All @@ -15,13 +17,13 @@
configure_celery(app)
celeryd_init.connect(validate_celery_config)

if app.conf['iib_otel_tracing']:
if os.getenv('IIB_OTEL_TRACING', '').lower() == 'true':
RequestsInstrumentor().instrument(trace_provider=tracerWrapper.provider)


# Add the init_celery_tracing method with its annotation
@worker_process_init.connect(weak=False)
def init_celery_tracing(*args, **kwargs):
"""Initialize the tracing for celery."""
if app.conf['iib_otel_tracing']:
if os.getenv('IIB_OTEL_TRACING', '').lower() == 'true':
CeleryInstrumentor().instrument(trace_provider=tracerWrapper.provider)
44 changes: 13 additions & 31 deletions tests/test_web/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,37 +170,19 @@ def test_validate_api_config_failure_aws_s3_params(config, error_msg):
validate_api_config(config)


@pytest.mark.parametrize(
'config, error_msg',
(
(
{
'IIB_AWS_S3_BUCKET_NAME': None,
'IIB_REQUEST_LOGS_DIR': 'some-dir',
'IIB_REQUEST_RECURSIVE_RELATED_BUNDLES_DIR': 'some-dir',
'IIB_GREENWAVE_CONFIG': {},
'IIB_BINARY_IMAGE_CONFIG': {},
'IIB_OTEL_TRACING': True,
},
(
'"OTEL_EXPORTER_OTLP_ENDPOINT" and "OTEL_SERVICE_NAME" environment '
'variables must be set to valid strings when IIB_OTEL_TRACING is set to True.'
),
),
(
{
'IIB_AWS_S3_BUCKET_NAME': None,
'IIB_REQUEST_LOGS_DIR': 'some-dir',
'IIB_REQUEST_RECURSIVE_RELATED_BUNDLES_DIR': 'some-dir',
'IIB_GREENWAVE_CONFIG': {},
'IIB_BINARY_IMAGE_CONFIG': {},
'IIB_OTEL_TRACING': 'some-str',
},
'"IIB_OTEL_TRACING" must be a valid boolean value',
),
),
)
def test_validate_api_config_failure_otel_params(config, error_msg):
@mock.patch.dict(os.environ, {'IIB_OTEL_TRACING': 'True'})
def test_validate_api_config_failure_otel_params():
config = {
'IIB_AWS_S3_BUCKET_NAME': None,
'IIB_REQUEST_LOGS_DIR': 'some-dir',
'IIB_REQUEST_RECURSIVE_RELATED_BUNDLES_DIR': 'some-dir',
'IIB_GREENWAVE_CONFIG': {},
'IIB_BINARY_IMAGE_CONFIG': {},
}
error_msg = (
'"OTEL_EXPORTER_OTLP_ENDPOINT" and "OTEL_SERVICE_NAME" environment '
'variables must be set to valid strings when IIB_OTEL_TRACING is set to True.'
)
with pytest.raises(ConfigError, match=error_msg):
validate_api_config(config)

Expand Down
28 changes: 9 additions & 19 deletions tests/test_workers/test_config.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-3.0-or-later
from unittest.mock import patch
from unittest import mock
from io import BytesIO
import os
import re
Expand Down Expand Up @@ -37,6 +38,7 @@ def test_configure_celery_with_classes_and_files(mock_open, mock_isfile, mock_ge
assert celery_app.conf.broker_connection_max_retries == 10


@mock.patch.dict(os.environ, {'IIB_OTEL_TRACING': 'some-str'})
@patch('os.path.isdir', return_value=True)
@patch('os.access', return_value=True)
def test_validate_celery_config(mock_isdir, mock_isaccess):
Expand Down Expand Up @@ -291,35 +293,23 @@ def test_validate_celery_config_invalid_s3_env_vars():
validate_celery_config(conf)


@pytest.mark.parametrize(
'config, error',
(
(
{'iib_otel_tracing': True},
(
'"OTEL_EXPORTER_OTLP_ENDPOINT" and "OTEL_SERVICE_NAME" environment '
'variables must be set to valid strings when iib_otel_tracing is set to True.'
),
),
(
{'iib_otel_tracing': 'random-str'},
lipoja marked this conversation as resolved.
Show resolved Hide resolved
'"iib_otel_tracing" must be a valid boolean value',
),
),
)
def test_validate_celery_config_invalid_otel_config(tmpdir, config, error):
@mock.patch.dict(os.environ, {'IIB_OTEL_TRACING': 'True'})
def test_validate_celery_config_invalid_otel_config(tmpdir):
conf = {
'iib_api_url': 'http://localhost:8080/api/v1/',
'iib_registry': 'registry',
'iib_required_labels': {},
'iib_organization_customizations': {},
'iib_request_recursive_related_bundles_dir': tmpdir.join('some-dir'),
}
error = (
'"OTEL_EXPORTER_OTLP_ENDPOINT" and "OTEL_SERVICE_NAME" environment '
'variables must be set to valid strings when "IIB_OTEL_TRACING" is set to True.'
)
iib_request_recursive_related_bundles_dir = conf['iib_request_recursive_related_bundles_dir']
iib_request_recursive_related_bundles_dir.mkdir()
worker_config = {**conf, **config}
with pytest.raises(ConfigError, match=error):
validate_celery_config(worker_config)
validate_celery_config(conf)


def test_validate_celery_config_invalid_recursive_related_bundles_config():
Expand Down