-
Notifications
You must be signed in to change notification settings - Fork 651
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
Make the configuration object universal #563
Changes from all commits
9b7cf01
a0ff175
d21666e
1db9a71
2eb48ad
099fa64
e33dcb3
5ff9200
ff662b9
93f899d
70d32f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,12 +18,42 @@ | |
""" | ||
Simple configuration manager | ||
|
||
This is a configuration manager for the Tracer and Meter providers. It reads | ||
configuration from environment variables prefixed with | ||
``OPENTELEMETRY_PYTHON_``: | ||
This is a configuration manager for OpenTelemetry. It reads configuration | ||
values from environment variables prefixed with | ||
``OPENTELEMETRY_PYTHON_`` whose characters are only all caps and underscores. | ||
The first character after ``OPENTELEMETRY_PYTHON_`` must be an uppercase | ||
character. | ||
|
||
1. ``OPENTELEMETRY_PYTHON_TRACER_PROVIDER`` | ||
2. ``OPENTELEMETRY_PYTHON_METER_PROVIDER`` | ||
For example, these environment variables will be read: | ||
|
||
1. ``OPENTELEMETRY_PYTHON_SOMETHING`` | ||
2. ``OPENTELEMETRY_PYTHON_SOMETHING_ELSE_`` | ||
3. ``OPENTELEMETRY_PYTHON_SOMETHING_ELSE_AND__ELSE`` | ||
|
||
These won't: | ||
|
||
1. ``OPENTELEMETRY_PYTH_SOMETHING`` | ||
2. ``OPENTELEMETRY_PYTHON_something`` | ||
3. ``OPENTELEMETRY_PYTHON_SOMETHING_2_AND__ELSE`` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any specific reason to not support this one? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed, any valid variable name is supported now. |
||
4. ``OPENTELEMETRY_PYTHON_SOMETHING_%_ELSE`` | ||
|
||
The values stored in the environment variables can be found in an instance of | ||
``opentelemetry.configuration.Configuration``. This class can be instantiated | ||
freely because instantiating it returns a singleton. | ||
|
||
For example, if the environment variable | ||
``OPENTELEMETRY_PYTHON_METER_PROVIDER`` value is ``my_meter_provider``, then | ||
``Configuration().meter_provider == "my_meter_provider"`` would be ``True``. | ||
|
||
Non defined attributes will always return ``None``. This is intended to make it | ||
easier to use the ``Configuration`` object in actual code, because it won't be | ||
necessary to check for the attribute to be defined first. | ||
|
||
Environment variables used by OpenTelemetry | ||
------------------------------------------- | ||
|
||
1. OPENTELEMETRY_PYTHON_METER_PROVIDER | ||
2. OPENTELEMETRY_PYTHON_TRACER_PROVIDER | ||
|
||
The value of these environment variables should be the name of the entry point | ||
that points to the class that implements either provider. This OpenTelemetry | ||
|
@@ -47,85 +77,46 @@ | |
"default_meter_provider" (this is not actually necessary since the | ||
OpenTelemetry API provided providers are the default ones used if no | ||
configuration is found in the environment variables). | ||
|
||
Once this is done, the configuration manager can be used by simply importing | ||
it from opentelemetry.configuration.Configuration. This is a class that can | ||
be instantiated as many times as needed without concern because it will | ||
always produce the same instance. Its attributes are lazy loaded and they | ||
hold an instance of their corresponding provider. So, for example, to get | ||
the configured meter provider:: | ||
|
||
from opentelemetry.configuration import Configuration | ||
|
||
tracer_provider = Configuration().tracer_provider | ||
|
||
""" | ||
|
||
from logging import getLogger | ||
from os import environ | ||
|
||
from pkg_resources import iter_entry_points | ||
|
||
logger = getLogger(__name__) | ||
from re import fullmatch | ||
|
||
|
||
class Configuration: | ||
_instance = None | ||
|
||
__slots__ = ("tracer_provider", "meter_provider") | ||
__slots__ = [] | ||
|
||
def __new__(cls) -> "Configuration": | ||
if Configuration._instance is None: | ||
|
||
configuration = { | ||
key: "default_{}".format(key) for key in cls.__slots__ | ||
} | ||
|
||
for key, value in configuration.items(): | ||
configuration[key] = environ.get( | ||
"OPENTELEMETRY_PYTHON_{}".format(key.upper()), value | ||
) | ||
|
||
for key, value in configuration.items(): | ||
underscored_key = "_{}".format(key) | ||
|
||
setattr(Configuration, underscored_key, None) | ||
setattr( | ||
Configuration, | ||
key, | ||
property( | ||
fget=lambda cls, local_key=key, local_value=value: cls._load( | ||
key=local_key, value=local_value | ||
) | ||
), | ||
) | ||
for key, value in environ.items(): | ||
|
||
match = fullmatch("OPENTELEMETRY_PYTHON_([A-Z][A-Z_]*)", key) | ||
|
||
if match is not None: | ||
|
||
key = match.group(1).lower() | ||
|
||
setattr(Configuration, "_{}".format(key), value) | ||
setattr( | ||
Configuration, | ||
key, | ||
property( | ||
fget=lambda cls, key=key: getattr( | ||
cls, "_{}".format(key) | ||
) | ||
), | ||
) | ||
|
||
Configuration.__slots__.append(key) | ||
|
||
Configuration.__slots__ = tuple(Configuration.__slots__) | ||
|
||
Configuration._instance = object.__new__(cls) | ||
|
||
return cls._instance | ||
|
||
@classmethod | ||
def _load(cls, key=None, value=None): | ||
underscored_key = "_{}".format(key) | ||
|
||
if getattr(cls, underscored_key) is None: | ||
try: | ||
setattr( | ||
cls, | ||
underscored_key, | ||
next( | ||
iter_entry_points( | ||
"opentelemetry_{}".format(key), name=value, | ||
) | ||
).load()(), | ||
) | ||
except Exception: # pylint: disable=broad-except | ||
# FIXME Decide on how to handle this. Should an exception be | ||
# raised here, or only a message should be logged and should | ||
# we fall back to the default meter provider? | ||
logger.error( | ||
"Failed to load configured provider %s", value, | ||
) | ||
raise | ||
|
||
return getattr(cls, underscored_key) | ||
def __getattr__(self, name): | ||
return None |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,104 +11,51 @@ | |
# 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. | ||
# pylint: disable-all | ||
|
||
from json import dumps | ||
from unittest import TestCase | ||
from unittest.mock import patch | ||
|
||
from pytest import fixture # type: ignore # pylint: disable=import-error | ||
|
||
from opentelemetry.configuration import Configuration # type: ignore | ||
|
||
|
||
class TestConfiguration(TestCase): | ||
class IterEntryPointsMock: | ||
def __init__( | ||
self, argument, name=None | ||
): # pylint: disable=unused-argument | ||
self._name = name | ||
|
||
def __next__(self): | ||
return self | ||
|
||
def __call__(self): | ||
return self._name | ||
|
||
def load(self): | ||
return self | ||
|
||
@fixture(autouse=True) | ||
def configdir(self, tmpdir): # type: ignore # pylint: disable=no-self-use | ||
tmpdir.chdir() | ||
tmpdir.mkdir(".config").join("opentelemetry_python.json").write( | ||
dumps({"tracer_provider": "overridden_tracer_provider"}) | ||
) | ||
|
||
def setUp(self): | ||
Configuration._instance = None # pylint: disable=protected-access | ||
from opentelemetry.configuration import Configuration # type: ignore | ||
|
||
def tearDown(self): | ||
Configuration._instance = None # pylint: disable=protected-access | ||
from opentelemetry.configuration import Configuration # type: ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this supposed to work? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a comment. |
||
|
||
def test_singleton(self): | ||
self.assertIsInstance(Configuration(), Configuration) | ||
self.assertIs(Configuration(), Configuration()) | ||
|
||
@patch( | ||
"opentelemetry.configuration.iter_entry_points", | ||
**{"side_effect": IterEntryPointsMock} # type: ignore | ||
) | ||
def test_lazy( # type: ignore | ||
self, mock_iter_entry_points, # pylint: disable=unused-argument | ||
): | ||
configuration = Configuration() | ||
|
||
self.assertIsNone( | ||
configuration._tracer_provider # pylint: disable=no-member,protected-access | ||
) | ||
|
||
configuration.tracer_provider # pylint: disable=pointless-statement | ||
|
||
self.assertEqual( | ||
configuration._tracer_provider, # pylint: disable=no-member,protected-access | ||
"default_tracer_provider", | ||
) | ||
|
||
@patch( | ||
"opentelemetry.configuration.iter_entry_points", | ||
**{"side_effect": IterEntryPointsMock} # type: ignore | ||
@patch.dict( | ||
"os.environ", # type: ignore | ||
{ | ||
"OPENTELEMETRY_PYTHON_METER_PROVIDER": "meter_provider", | ||
"OPENTELEMETRY_PYTHON_TRACER_PROVIDER": "tracer_provider", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be nice to have other env variables here to show that it's generic. |
||
}, | ||
) | ||
def test_default_values( # type: ignore | ||
self, mock_iter_entry_points # pylint: disable=unused-argument | ||
): | ||
def test_environment_variables(self): # type: ignore | ||
self.assertEqual( | ||
Configuration().tracer_provider, "default_tracer_provider" | ||
Configuration().meter_provider, "meter_provider" | ||
) # pylint: disable=no-member | ||
self.assertEqual( | ||
Configuration().meter_provider, "default_meter_provider" | ||
Configuration().tracer_provider, "tracer_provider" | ||
) # pylint: disable=no-member | ||
|
||
@patch( | ||
"opentelemetry.configuration.iter_entry_points", | ||
**{"side_effect": IterEntryPointsMock} # type: ignore | ||
) | ||
@patch.dict( | ||
"os.environ", | ||
{"OPENTELEMETRY_PYTHON_METER_PROVIDER": "overridden_meter_provider"}, | ||
"os.environ", # type: ignore | ||
{"OPENTELEMETRY_PYTHON_TRACER_PROVIDER": "tracer_provider"}, | ||
) | ||
def test_environment_variables( # type: ignore | ||
self, mock_iter_entry_points # pylint: disable=unused-argument | ||
): # type: ignore | ||
self.assertEqual( | ||
Configuration().tracer_provider, "default_tracer_provider" | ||
) # pylint: disable=no-member | ||
self.assertEqual( | ||
Configuration().meter_provider, "overridden_meter_provider" | ||
) # pylint: disable=no-member | ||
|
||
def test_property(self): | ||
with self.assertRaises(AttributeError): | ||
Configuration().tracer_provider = "new_tracer_provider" | ||
|
||
def test_slots(self): | ||
with self.assertRaises(AttributeError): | ||
Configuration().xyz = "xyz" # pylint: disable=assigning-non-slot | ||
|
||
def test_getattr(self): | ||
Configuration().xyz is None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it missing the assertion? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Iep, 😅 fixing... |
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 this sentence is not needed as the sentence before already implies it.
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.