Skip to content

Commit

Permalink
Upgraded to Otel 1.5.0 (#116)
Browse files Browse the repository at this point in the history
- Dropped SPLUNK_MAX_ATTR_LENGTH env var (was undocumented).
- Dropped SPLUNK_SERVICE_NAME env var.
  • Loading branch information
owais authored Aug 28, 2021
1 parent 7b0cc8a commit c564659
Show file tree
Hide file tree
Showing 12 changed files with 217 additions and 190 deletions.
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,18 +1,33 @@
# Changelog

## [Unreleased]
### Changed

- Upgraded Otel dependencies to 1.5.0 and 0.24.0b0

### Removed

- SPLUNK_SERVICE_NAME and SPLUNK_MAX_ATTR_LENGTH env vars were removed.
Use `OTEL_SERVICE_NAME` and `OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT` instead.
[#116](https://github.com/signalfx/splunk-otel-python/pull/116)

## [0.16.0] - 2021-08-04

### Changed

- Upgraded Otel dependencies to 1.4.1 and 0.23.0b2

## [1.0.0rc3] - 2021-06-08

### Changed

- Pin exact Otel deps until 1.0
[#88](https://github.com/signalfx/splunk-otel-python/pull/88)

## [1.0.0-rc2] - 2021-06-03

### Changed

- Upgrade OpenTelemetry Python to 1.3.0 and 0.22b0
[#85](https://github.com/signalfx/splunk-otel-python/pull/85)

Expand Down
263 changes: 154 additions & 109 deletions poetry.lock

Large diffs are not rendered by default.

13 changes: 7 additions & 6 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@ splunk_distro = "splunk_otel.distro:SplunkDistro"

[tool.poetry.dependencies]
python = "^3.6"
opentelemetry-api = "1.4.1"
opentelemetry-sdk = "1.4.1"
opentelemetry-instrumentation = "0.23b2"
opentelemetry-propagator-b3 = {version = "1.4.1", optional = true}
opentelemetry-exporter-jaeger-thrift = {version = "1.4.1", optional = true}
opentelemetry-exporter-otlp-proto-grpc = {version = "1.4.1", optional = true}
opentelemetry-api = "1.5.0"
opentelemetry-sdk = "1.5.0"
opentelemetry-instrumentation = "0.24b0"
opentelemetry-semantic-conventions = "0.24b0"
opentelemetry-propagator-b3 = {version = "1.5.0", optional = true}
opentelemetry-exporter-jaeger-thrift = {version = "1.5.0", optional = true}
opentelemetry-exporter-otlp-proto-grpc = {version = "1.5.0", optional = true}

[tool.poetry.extras]
all = ["opentelemetry-propagator-b3", "opentelemetry-exporter-otlp-proto-grpc", "opentelemetry-exporter-jaeger-thrift"]
Expand Down
2 changes: 1 addition & 1 deletion scripts/create_gh_release.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def render_changelog(version):


def render_release_notes(**kwargs):
with open(relase_template_path, "r") as tmpl:
with open(relase_template_path, "r", encoding="utf-8") as tmpl:
notes = tmpl.read().format(**kwargs)
return notes

Expand Down
4 changes: 2 additions & 2 deletions scripts/prepare_release.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ def update_docs(versions):
}

markdown = ""
with open(readme_path, "r") as readme:
with open(readme_path, "r", encoding="utf-8") as readme:
markdown = readme.read()

for name, value in variables.items():
markdown = re.sub(regexp.format(name), value, markdown)

with open(readme_path, "w") as readme:
with open(readme_path, "w", encoding="utf-8") as readme:
readme.write(markdown)


Expand Down
2 changes: 1 addition & 1 deletion splunk_otel/cmd/trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def run() -> None:
os.environ["SPLUNK_ACCESS_TOKEN"] = args.token

if args.service_name:
os.environ["SPLUNK_SERVICE_NAME"] = args.service_name
os.environ["OTEL_SERVICE_NAME"] = args.service_name

if not args.command:
ap.error(ap.format_help())
Expand Down
6 changes: 2 additions & 4 deletions splunk_otel/environment_variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,5 @@
# See the License for the specific language governing permissions and
# limitations under the License.

SPLUNK_ACCESS_TOKEN = "SPLUNK_ACCESS_TOKEN"
SPLUNK_MAX_ATTR_LENGTH = "SPLUNK_MAX_ATTR_LENGTH"
SPLUNK_TRACE_RESPONSE_HEADER_ENABLED = "SPLUNK_TRACE_RESPONSE_HEADER_ENABLED"
SPLUNK_SERVICE_NAME = "SPLUNK_SERVICE_NAME"
_SPLUNK_ACCESS_TOKEN = "SPLUNK_ACCESS_TOKEN"
_SPLUNK_TRACE_RESPONSE_HEADER_ENABLED = "SPLUNK_TRACE_RESPONSE_HEADER_ENABLED"
73 changes: 25 additions & 48 deletions splunk_otel/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@
__version__ as auto_instrumentation_version,
)
from opentelemetry.sdk.environment_variables import (
OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT,
OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT,
OTEL_EXPORTER_JAEGER_ENDPOINT,
OTEL_SERVICE_NAME,
OTEL_LINK_ATTRIBUTE_COUNT_LIMIT,
OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT,
OTEL_SPAN_EVENT_COUNT_LIMIT,
OTEL_SPAN_LINK_COUNT_LIMIT,
Expand All @@ -34,10 +36,8 @@
from pkg_resources import iter_entry_points

from splunk_otel.environment_variables import (
SPLUNK_ACCESS_TOKEN,
SPLUNK_MAX_ATTR_LENGTH,
SPLUNK_SERVICE_NAME,
SPLUNK_TRACE_RESPONSE_HEADER_ENABLED,
_SPLUNK_ACCESS_TOKEN,
_SPLUNK_TRACE_RESPONSE_HEADER_ENABLED,
)
from splunk_otel.propagators import ServerTimingResponsePropagator
from splunk_otel.symbols import (
Expand Down Expand Up @@ -69,7 +69,6 @@
class _Options:
span_exporter_factories: Collection[_SpanExporterFactory]
access_token: Optional[str]
max_attr_length: int
resource: Resource
response_propagation: bool
response_propagator: Optional[ResponsePropagator]
Expand All @@ -79,13 +78,11 @@ def __init__(
service_name: Optional[str] = None,
span_exporter_factories: Optional[Collection[_SpanExporterFactory]] = None,
access_token: Optional[str] = None,
max_attr_length: Optional[int] = None,
resource_attributes: Optional[Dict[str, Union[str, bool, int, float]]] = None,
trace_response_header_enabled: Optional[bool] = None,
):
self._set_default_env()
self.access_token = self._get_access_token(access_token)
self.max_attr_length = self._get_max_attr_length(max_attr_length)
self.response_propagator = self._get_trace_response_header_enabled(
trace_response_header_enabled
)
Expand All @@ -97,27 +94,16 @@ def __init__(
@staticmethod
def _get_access_token(access_token: Optional[str]) -> Optional[str]:
if not access_token:
access_token = environ.get(SPLUNK_ACCESS_TOKEN)
access_token = environ.get(_SPLUNK_ACCESS_TOKEN)
return access_token or None

@staticmethod
def _get_max_attr_length(max_attr_length: Optional[int]) -> int:
if not max_attr_length:
value = environ.get(SPLUNK_MAX_ATTR_LENGTH)
if value:
try:
max_attr_length = int(value)
except (TypeError, ValueError):
logger.error("SPLUNK_MAX_ATTR_LENGTH must be a number.")
return max_attr_length or _DEFAULT_MAX_ATTR_LENGTH

@staticmethod
def _get_trace_response_header_enabled(
enabled: Optional[bool],
) -> Optional[ResponsePropagator]:
if enabled is None:
enabled = _Options._is_truthy(
environ.get(SPLUNK_TRACE_RESPONSE_HEADER_ENABLED, "true")
environ.get(_SPLUNK_TRACE_RESPONSE_HEADER_ENABLED, "true")
)
if enabled:
return ServerTimingResponsePropagator()
Expand Down Expand Up @@ -172,20 +158,19 @@ def _is_truthy(cls, value: Optional[str]) -> bool:

@staticmethod
def _set_default_env() -> None:
otel_service_name = environ.get(OTEL_SERVICE_NAME, "")
splunk_service_name = environ.get(SPLUNK_SERVICE_NAME)
if not otel_service_name and splunk_service_name:
logger.warning(
"SPLUNK_SERVICE_NAME is deprecated and will be removed soon. Please use OTEL_SERVICE_NAME instead"
)
environ[OTEL_SERVICE_NAME] = splunk_service_name
defaults = {
OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT: _LIMIT_UNSET_VALUE,
OTEL_SPAN_EVENT_COUNT_LIMIT: _LIMIT_UNSET_VALUE,
OTEL_SPAN_EVENT_COUNT_LIMIT: _LIMIT_UNSET_VALUE,
OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT: _LIMIT_UNSET_VALUE,
OTEL_LINK_ATTRIBUTE_COUNT_LIMIT: _LIMIT_UNSET_VALUE,
OTEL_SPAN_LINK_COUNT_LIMIT: str(_DEFAULT_SPAN_LINK_COUNT_LIMIT),
OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT: str(_DEFAULT_MAX_ATTR_LENGTH),
}

if OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT not in environ:
environ[OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT] = _LIMIT_UNSET_VALUE
if OTEL_SPAN_EVENT_COUNT_LIMIT not in environ:
environ[OTEL_SPAN_EVENT_COUNT_LIMIT] = _LIMIT_UNSET_VALUE
if OTEL_SPAN_LINK_COUNT_LIMIT not in environ:
environ[OTEL_SPAN_LINK_COUNT_LIMIT] = str(_DEFAULT_SPAN_LINK_COUNT_LIMIT)
for key, value in defaults.items():
if key not in environ:
environ[key] = value

@classmethod
def _get_span_exporter_names_from_env(cls) -> Collection[Tuple[str, str]]:
Expand Down Expand Up @@ -276,22 +261,14 @@ def _jaeger_factory(
return exporter(**_Options._get_jaeger_kwargs(options))

@staticmethod
def _get_jaeger_kwargs(options: "_Options") -> Dict[str, Union[int, str]]:
kwargs: Dict[str, Union[int, str]] = {
"max_tag_value_length": options.max_attr_length,
}
def _get_jaeger_kwargs(options: "_Options") -> Dict[str, str]:
if options.access_token:
kwargs.update(
{
"username": "auth",
"password": options.access_token,
}
)
return kwargs
return {
"username": "auth",
"password": options.access_token,
}
return {}

@staticmethod
def _otlp_factory(exporter: _SpanExporterClass, options: "_Options") -> SpanExporter:
# TODO: enable after PR is merged and released:
# https://github.com/open-telemetry/opentelemetry-python/pull/1824
# kwargs = {"max_attr_value_length": self.max_attr_length}
return exporter(headers=(("x-sf-token", options.access_token),))
2 changes: 1 addition & 1 deletion splunk_otel/symbols.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@
_EXPORTER_JAEGER_SPLUNK: "opentelemetry-exporter-jaeger-thrift",
}

_LIMIT_UNSET_VALUE = "unset"
_LIMIT_UNSET_VALUE = ""
2 changes: 0 additions & 2 deletions splunk_otel/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ def start_tracing(
service_name: Optional[str] = None,
span_exporter_factories: Optional[Collection[_SpanExporterFactory]] = None,
access_token: Optional[str] = None,
max_attr_length: Optional[int] = None,
resource_attributes: Optional[Dict[str, Union[str, bool, int, float]]] = None,
trace_response_header_enabled: Optional[bool] = None,
) -> None:
Expand All @@ -47,7 +46,6 @@ def start_tracing(
service_name,
span_exporter_factories,
access_token,
max_attr_length,
resource_attributes,
trace_response_header_enabled,
)
Expand Down
17 changes: 1 addition & 16 deletions tests/unit/test_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from opentelemetry.sdk.trace.export import ConsoleSpanExporter

from splunk_otel.options import _Options
from splunk_otel.symbols import _DEFAULT_JAEGER_ENDPOINT, _DEFAULT_MAX_ATTR_LENGTH
from splunk_otel.symbols import _DEFAULT_JAEGER_ENDPOINT
from splunk_otel.version import __version__

# pylint: disable=protected-access
Expand Down Expand Up @@ -67,18 +67,6 @@ def test_service_name_from_env_service_name(self):
"service name from otel service name env",
)

@mock.patch.dict(
os.environ,
{"SPLUNK_SERVICE_NAME": "service name from splunk env"},
)
def test_service_name_backward_compatibility(self):
self.assertNotIn("OTEL_SERVICE_NAME", os.environ)
_Options()
self.assertEqual(
os.environ["OTEL_SERVICE_NAME"], os.environ["SPLUNK_SERVICE_NAME"]
)
del os.environ["OTEL_SERVICE_NAME"]

@mock.patch.dict(os.environ, {"OTEL_TRACES_EXPORTER": ""})
def test_exporters_default(self):
options = _Options()
Expand Down Expand Up @@ -130,14 +118,12 @@ def test_exporters_jaeger_defaults(self):
exporter = factory(options)
self.assertIsInstance(exporter, JaegerExporter)
self.assertEqual(exporter.collector_endpoint, _DEFAULT_JAEGER_ENDPOINT)
self.assertEqual(exporter._max_tag_value_length, _DEFAULT_MAX_ATTR_LENGTH)

@mock.patch.dict(
os.environ,
{
"OTEL_TRACES_EXPORTER": "jaeger-thrift-splunk",
"OTEL_EXPORTER_JAEGER_ENDPOINT": "localhost:1234",
"SPLUNK_MAX_ATTR_LENGTH": "10",
"SPLUNK_ACCESS_TOKEN": "12345",
},
)
Expand All @@ -148,7 +134,6 @@ def test_exporters_jaeger_custom(self):
exporter = factory(options)
self.assertIsInstance(exporter, JaegerExporter)
self.assertEqual(exporter.collector_endpoint, "localhost:1234")
self.assertEqual(exporter._max_tag_value_length, 10)
self.assertEqual(exporter.username, "auth")
self.assertEqual(exporter.password, "12345")

Expand Down
8 changes: 8 additions & 0 deletions tests/unit/test_otel_defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,17 @@ def test_default_limits(self):
# instantiating _Options() sets default env vars
_Options()
limits = TracerProvider()._span_limits

# unlimited by default
self.assertIsNone(limits.max_attributes)
self.assertIsNone(limits.max_events)
self.assertIsNone(limits.max_event_attributes)
self.assertIsNone(limits.max_link_attributes)

# have default limits
self.assertEqual(limits.max_links, 1000)
self.assertEqual(limits.max_attribute_length, 1200)
self.assertEqual(limits.max_span_attribute_length, 1200)

def test_otel_log_correlation_enabled(self):
self.assertTrue(environ[_OTEL_PYTHON_LOG_CORRELATION])

0 comments on commit c564659

Please sign in to comment.