diff --git a/README.md b/README.md index a95bcfd5a..3b7ec4213 100644 --- a/README.md +++ b/README.md @@ -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 @@ -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" @@ -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 @@ -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" diff --git a/iib/common/tracing.py b/iib/common/tracing.py index 82d0ecd25..c84318b24 100644 --- a/iib/common/tracing.py +++ b/iib/common/tracing.py @@ -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() @@ -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: @@ -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'): diff --git a/iib/web/app.py b/iib/web/app.py index 9af852e16..933953264 100644 --- a/iib/web/app.py +++ b/iib/web/app.py @@ -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 bool(os.getenv('IIB_OTEL_TRACING', False)): 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 ' @@ -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 bool(os.getenv('IIB_OTEL_TRACING', False)): FlaskInstrumentor().instrument_app( app, enable_commenter=True, commenter_options={}, tracer_provider=tracerWrapper.provider ) diff --git a/iib/web/config.py b/iib/web/config.py index 66dc7a641..0029578d0 100644 --- a/iib/web/config.py +++ b/iib/web/config.py @@ -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 diff --git a/iib/workers/config.py b/iib/workers/config.py index 387aa8b15..703401ea3 100644 --- a/iib/workers/config.py +++ b/iib/workers/config.py @@ -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 = ( @@ -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.' ) diff --git a/iib/workers/tasks/celery.py b/iib/workers/tasks/celery.py index 9fe15f627..585f7258f 100644 --- a/iib/workers/tasks/celery.py +++ b/iib/workers/tasks/celery.py @@ -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 @@ -15,7 +17,7 @@ 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) @@ -23,5 +25,5 @@ @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) diff --git a/tests/test_web/test_app.py b/tests/test_web/test_app.py index afefb7cf3..8545bfdd1 100644 --- a/tests/test_web/test_app.py +++ b/tests/test_web/test_app.py @@ -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) diff --git a/tests/test_workers/test_config.py b/tests/test_workers/test_config.py index a7fe9aa3f..b6bbbd266 100644 --- a/tests/test_workers/test_config.py +++ b/tests/test_workers/test_config.py @@ -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 @@ -291,23 +292,8 @@ 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'}, - '"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', @@ -315,11 +301,14 @@ def test_validate_celery_config_invalid_otel_config(tmpdir, config, error): '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():