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

Implement CreateKey Functionality #1853

Merged
merged 45 commits into from
Jun 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
6afb10c
Add create key functionality
euniceek May 14, 2021
edbe22b
Update unit tests
euniceek May 14, 2021
1ec001d
Update create_key comment
euniceek May 14, 2021
9010487
Change key to reflect key name
euniceek May 15, 2021
95af93d
Update CHANGELOG
euniceek May 17, 2021
fb86f6f
Update set_value inputs to key
euniceek May 17, 2021
1f59f5b
Fix lint errors
euniceek May 17, 2021
a939327
Add create key functionality
euniceek May 14, 2021
20257df
Update unit tests
euniceek May 14, 2021
a8741ab
Update create_key comment
euniceek May 14, 2021
657e219
Change key to reflect key name
euniceek May 15, 2021
065a3c5
Update CHANGELOG
euniceek May 17, 2021
669544b
Update set_value inputs to key
euniceek May 17, 2021
43b60d3
Fix lint errors
euniceek May 17, 2021
5ceae8e
Update contrib repo git SHA
euniceek May 21, 2021
c00af0a
Merge branch 'open-telemetry:main' into create-context-key
euniceek May 21, 2021
df7c79c
Update contrib repo SHA
euniceek May 21, 2021
0350096
Merge branch 'open-telemetry:main' into create-context-key
euniceek May 24, 2021
78a4610
Merge branch 'main' of https://github.com/open-o11y/opentelemetry-pyt…
euniceek May 25, 2021
daa0de2
Rename export key to suppress instrumentation key
euniceek May 25, 2021
9423750
Merge branch 'open-telemetry:main' into create-context-key
euniceek May 25, 2021
1be14b9
Merge branch 'create-context-key' of https://github.com/open-o11y/ope…
euniceek May 25, 2021
4af00ed
Merge branch 'open-telemetry:main' into create-context-key
euniceek May 26, 2021
95c0270
Merge branch 'open-telemetry:main' into create-context-key
euniceek May 31, 2021
3a1956f
Move suppress key to instrumentation package
euniceek May 31, 2021
88c430e
Update SHA and fix lint error
euniceek May 31, 2021
c9df01f
Update tox.ini
euniceek Jun 1, 2021
f0d5437
Update tox.ini for opencensus
euniceek Jun 1, 2021
800387c
Merge branch 'open-telemetry:main' into create-context-key
euniceek Jun 1, 2021
d91d2ba
Merge remote-tracking branch 'upstream/main' into create-context-key
euniceek Jun 2, 2021
a14315b
Add opentelemtry-instrumentation to sdk setup.cfg
euniceek Jun 3, 2021
85d569b
Merge remote-tracking branch 'upstream/main' into create-context-key
euniceek Jun 6, 2021
061f591
Fix tox.ini
euniceek Jun 6, 2021
8e2d3a3
Change instrumentation order in tox.ini
euniceek Jun 6, 2021
c4cdffd
Add instrumentation to docs-requirements
euniceek Jun 6, 2021
268f268
Update SHA
euniceek Jun 6, 2021
5dae28c
Merge branch 'open-telemetry:main' into create-context-key
euniceek Jun 7, 2021
38ca88e
Fix public API errors
euniceek Jun 7, 2021
4cf1bf5
Update contrib sha and tox.ini
euniceek Jun 7, 2021
ba15cdc
Change context import order
euniceek Jun 7, 2021
092e341
Fix lint errors
euniceek Jun 10, 2021
eaff5b2
Revert public API errors
euniceek Jun 11, 2021
9a45def
Merge branch 'open-telemetry:main' into create-context-key
euniceek Jun 11, 2021
4ed8eea
Fix span_key and shim_key
euniceek Jun 11, 2021
7d67ff9
Merge branch 'open-telemetry:main' into create-context-key
euniceek Jun 11, 2021
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
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ env:
# Otherwise, set variable to the commit of your branch on
# opentelemetry-python-contrib which is compatible with these Core repo
# changes.
CONTRIB_REPO_SHA: 01bb63fb8cc429a17f1fb0c93f7ac72a3fc7b4fc
CONTRIB_REPO_SHA: 82c4f600872a72fedaebad758f0d5588dfb53a00

jobs:
build:
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added
- Allow span limits to be set programatically via TracerProvider.
([#1877](https://github.com/open-telemetry/opentelemetry-python/pull/1877))
- Added support for CreateKey functionality.
euniceek marked this conversation as resolved.
Show resolved Hide resolved
([#1853](https://github.com/open-telemetry/opentelemetry-python/pull/1853))

### Changed
- Updated get_tracer to return an empty string when passed an invalid name
Expand Down
1 change: 1 addition & 0 deletions docs-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ sphinx-jekyll-builder
# doesn't work for pkg_resources.
./opentelemetry-api
./opentelemetry-semantic-conventions
./opentelemetry-python-contrib/opentelemetry-instrumentation
./opentelemetry-sdk

# Required by instrumentation and exporter packages
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-api/src/opentelemetry/baggage/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
import typing
from types import MappingProxyType

from opentelemetry.context import get_value, set_value
from opentelemetry.context import create_key, get_value, set_value
from opentelemetry.context.context import Context

_BAGGAGE_KEY = "baggage"
_BAGGAGE_KEY = create_key("baggage")


def get_all(
Expand Down
13 changes: 13 additions & 0 deletions opentelemetry-api/src/opentelemetry/context/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import logging
import threading
import typing
import uuid
from functools import wraps
from os import environ

Expand Down Expand Up @@ -68,6 +69,18 @@ def wrapper( # type: ignore[misc]
return typing.cast(_F, wrapper) # type: ignore[misc]


def create_key(keyname: str) -> str:
"""To allow cross-cutting concern to control access to their local state,
the RuntimeContext API provides a function which takes a keyname as input,
and returns a unique key.
Args:
keyname: The key name is for debugging purposes and is not required to be unique.
Returns:
A unique string representing the newly created key.
"""
return keyname + "-" + str(uuid.uuid4())


def get_value(key: str, context: typing.Optional[Context] = None) -> "object":
"""To access the local state of a concern, the RuntimeContext API
provides a function which takes a context and a key as input,
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-api/src/opentelemetry/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
from opentelemetry.context.context import Context
from opentelemetry.environment_variables import OTEL_PYTHON_TRACER_PROVIDER
from opentelemetry.trace.propagation import (
SPAN_KEY,
_SPAN_KEY,
get_current_span,
set_span_in_context,
)
Expand Down Expand Up @@ -517,7 +517,7 @@ def use_span(
this mechanism if it was previously set manually.
"""
try:
token = context_api.attach(context_api.set_value(SPAN_KEY, span))
token = context_api.attach(context_api.set_value(_SPAN_KEY, span))
try:
yield span
finally:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@
# limitations under the License.
from typing import Optional

from opentelemetry.context import get_value, set_value
from opentelemetry.context import create_key, get_value, set_value
from opentelemetry.context.context import Context
from opentelemetry.trace.span import INVALID_SPAN, Span

SPAN_KEY = "current-span"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not a breaking API change? cc @ocelotl

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably this was changed to avoid the public api test cases from breaking, SPAN_KEY should remain public and the tests will pass after the corresponding label is added. @eunice98k can you please fix this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The public API check is passing on the current commit, shouldn't it have caught this? Or does it only check for additions atm?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only checks for additions. The script is not meant to detect breaking changes, but to warn us when a possibly unnecessarily new public symbol is being added, to avoid our API from growing without need.

_SPAN_KEY = create_key("current-span")


def set_span_in_context(
Expand All @@ -30,7 +31,7 @@ def set_span_in_context(
context: a Context object. if one is not passed, the
default current context is used instead.
"""
ctx = set_value(SPAN_KEY, span, context=context)
ctx = set_value(_SPAN_KEY, span, context=context)
return ctx


Expand All @@ -44,7 +45,7 @@ def get_current_span(context: Optional[Context] = None) -> Span:
Returns:
The Span set in the context if it exists. INVALID_SPAN otherwise.
"""
span = get_value(SPAN_KEY, context=context)
span = get_value(_SPAN_KEY, context=context)
if span is None or not isinstance(span, Span):
return INVALID_SPAN
return span
32 changes: 22 additions & 10 deletions opentelemetry-api/tests/context/test_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,39 @@
from opentelemetry.context.context import Context


def do_work() -> None:
context.attach(context.set_value("say", "bar"))
def _do_work() -> str:
key = context.create_key("say")
context.attach(context.set_value(key, "bar"))
return key


class TestContext(unittest.TestCase):
def setUp(self):
context.attach(Context())

def test_context_key(self):
key1 = context.create_key("say")
key2 = context.create_key("say")
self.assertNotEqual(key1, key2)
first = context.set_value(key1, "foo")
second = context.set_value(key2, "bar")
self.assertEqual(context.get_value(key1, context=first), "foo")
self.assertEqual(context.get_value(key2, context=second), "bar")

def test_context(self):
self.assertIsNone(context.get_value("say"))
key1 = context.create_key("say")
self.assertIsNone(context.get_value(key1))
empty = context.get_current()
second = context.set_value("say", "foo")
self.assertEqual(context.get_value("say", context=second), "foo")
second = context.set_value(key1, "foo")
self.assertEqual(context.get_value(key1, context=second), "foo")

do_work()
self.assertEqual(context.get_value("say"), "bar")
key2 = _do_work()
self.assertEqual(context.get_value(key2), "bar")
third = context.get_current()

self.assertIsNone(context.get_value("say", context=empty))
self.assertEqual(context.get_value("say", context=second), "foo")
self.assertEqual(context.get_value("say", context=third), "bar")
self.assertIsNone(context.get_value(key1, context=empty))
self.assertEqual(context.get_value(key1, context=second), "foo")
self.assertEqual(context.get_value(key2, context=third), "bar")

def test_set_value(self):
first = context.set_value("a", "yyy")
Expand Down
1 change: 1 addition & 0 deletions opentelemetry-sdk/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ include_package_data = True
install_requires =
opentelemetry-api == 1.4.0.dev0
opentelemetry-semantic-conventions == 0.23.dev0
opentelemetry-instrumentation == 0.23.dev0

[options.packages.find]
where = src
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from typing import Optional

from opentelemetry.context import Context, attach, detach, set_value
from opentelemetry.instrumentation.utils import _SUPPRESS_INSTRUMENTATION_KEY
euniceek marked this conversation as resolved.
Show resolved Hide resolved
from opentelemetry.sdk.environment_variables import (
OTEL_BSP_EXPORT_TIMEOUT,
OTEL_BSP_MAX_EXPORT_BATCH_SIZE,
Expand Down Expand Up @@ -86,7 +87,7 @@ def on_start(
def on_end(self, span: ReadableSpan) -> None:
if not span.context.trace_flags.sampled:
return
token = attach(set_value("suppress_instrumentation", True))
token = attach(set_value(_SUPPRESS_INSTRUMENTATION_KEY, True))
try:
self.span_exporter.export((span,))
# pylint: disable=broad-except
Expand Down Expand Up @@ -326,7 +327,7 @@ def _export_batch(self) -> int:
while idx < self.max_export_batch_size and self.queue:
self.spans_list[idx] = self.queue.pop()
idx += 1
token = attach(set_value("suppress_instrumentation", True))
token = attach(set_value(_SUPPRESS_INSTRUMENTATION_KEY, True))
try:
# Ignore type b/c the Optional[None]+slicing is too "clever"
# for mypy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
B3SingleFormat,
)
from opentelemetry.propagators.textmap import DefaultGetter
from opentelemetry.trace.propagation import _SPAN_KEY


def get_child_parent_new_carrier(old_carrier, propagator):
Expand Down Expand Up @@ -247,7 +248,7 @@ def test_derived_ctx_is_returned_for_success(self):
},
old_ctx,
)
self.assertIn("current-span", new_ctx)
self.assertIn(_SPAN_KEY, new_ctx)
for key, value in old_ctx.items(): # pylint:disable=no-member
self.assertIn(key, new_ctx)
# pylint:disable=unsubscriptable-object
Expand All @@ -257,7 +258,7 @@ def test_derived_ctx_is_returned_for_failure(self):
"""Ensure returned context is derived from the given context."""
old_ctx = Context({"k2": "v2"})
new_ctx = self.get_propagator().extract({}, old_ctx)
self.assertNotIn("current-span", new_ctx)
self.assertNotIn(_SPAN_KEY, new_ctx)
for key, value in old_ctx.items(): # pylint:disable=no-member
self.assertIn(key, new_ctx)
# pylint:disable=unsubscriptable-object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import opentelemetry.sdk.trace.id_generator as id_generator
import opentelemetry.trace as trace_api
from opentelemetry import baggage
from opentelemetry.baggage import _BAGGAGE_KEY
from opentelemetry.context import Context
from opentelemetry.propagators import ( # pylint: disable=no-name-in-module
jaeger,
Expand Down Expand Up @@ -134,15 +135,15 @@ def test_baggage(self):
input_baggage = {"key1": "value1"}
_, new_carrier = get_context_new_carrier(old_carrier, input_baggage)
ctx = FORMAT.extract(new_carrier)
self.assertDictEqual(input_baggage, ctx["baggage"])
self.assertDictEqual(input_baggage, ctx[_BAGGAGE_KEY])

def test_non_string_baggage(self):
old_carrier = {FORMAT.TRACE_ID_KEY: self.serialized_uber_trace_id}
input_baggage = {"key1": 1, "key2": True}
formatted_baggage = {"key1": "1", "key2": "True"}
_, new_carrier = get_context_new_carrier(old_carrier, input_baggage)
ctx = FORMAT.extract(new_carrier)
self.assertDictEqual(formatted_baggage, ctx["baggage"])
self.assertDictEqual(formatted_baggage, ctx[_BAGGAGE_KEY])

def test_extract_invalid_uber_trace_id(self):
old_carrier = {
Expand All @@ -153,7 +154,7 @@ def test_extract_invalid_uber_trace_id(self):
context = FORMAT.extract(old_carrier)
span_context = trace_api.get_current_span(context).get_span_context()
self.assertEqual(span_context.span_id, trace_api.INVALID_SPAN_ID)
self.assertDictEqual(formatted_baggage, context["baggage"])
self.assertDictEqual(formatted_baggage, context[_BAGGAGE_KEY])

def test_extract_invalid_trace_id(self):
old_carrier = {
Expand All @@ -164,7 +165,7 @@ def test_extract_invalid_trace_id(self):
context = FORMAT.extract(old_carrier)
span_context = trace_api.get_current_span(context).get_span_context()
self.assertEqual(span_context.trace_id, trace_api.INVALID_TRACE_ID)
self.assertDictEqual(formatted_baggage, context["baggage"])
self.assertDictEqual(formatted_baggage, context[_BAGGAGE_KEY])

def test_extract_invalid_span_id(self):
old_carrier = {
Expand All @@ -175,7 +176,7 @@ def test_extract_invalid_span_id(self):
context = FORMAT.extract(old_carrier)
span_context = trace_api.get_current_span(context).get_span_context()
self.assertEqual(span_context.span_id, trace_api.INVALID_SPAN_ID)
self.assertDictEqual(formatted_baggage, context["baggage"])
self.assertDictEqual(formatted_baggage, context[_BAGGAGE_KEY])

def test_fields(self):
tracer = trace.TracerProvider().get_tracer("sdk_tracer_provider")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,14 @@
)

from opentelemetry.baggage import get_baggage, set_baggage
from opentelemetry.context import Context, attach, detach, get_value, set_value
from opentelemetry.context import (
Context,
attach,
create_key,
detach,
get_value,
set_value,
)
from opentelemetry.propagate import get_global_textmap
from opentelemetry.shim.opentracing_shim import util
from opentelemetry.shim.opentracing_shim.version import __version__
Expand All @@ -118,6 +125,7 @@

ValueT = TypeVar("ValueT", int, float, bool, str)
logger = logging.getLogger(__name__)
_SHIM_KEY = create_key("scope_shim")


def create_tracer(otel_tracer_provider: TracerProvider) -> "TracerShim":
Expand Down Expand Up @@ -349,7 +357,7 @@ def __init__(
):
super().__init__(manager, span)
self._span_cm = span_cm
self._token = attach(set_value("scope_shim", self))
self._token = attach(set_value(_SHIM_KEY, self))

# TODO: Change type of `manager` argument to `opentracing.ScopeManager`? We
# need to get rid of `manager.tracer` for this.
Expand Down Expand Up @@ -486,7 +494,7 @@ def active(self) -> "ScopeShim":
return None

try:
return get_value("scope_shim")
return get_value(_SHIM_KEY)
except KeyError:
span_context = SpanContextShim(span.get_span_context())
wrapped_span = SpanShim(self._tracer, span_context, span)
Expand Down
11 changes: 8 additions & 3 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,10 @@ commands_pre =
py3{6,7,8,9}: python -m pip install -U pip setuptools wheel
; Install common packages for all the tests. These are not needed in all the
; cases but it saves a lot of boilerplate in this file.
test: pip install {toxinidir}/opentelemetry-api {toxinidir}/opentelemetry-semantic-conventions {toxinidir}/opentelemetry-sdk {toxinidir}/tests/util
test: pip install {toxinidir}/opentelemetry-api {toxinidir}/opentelemetry-semantic-conventions {toxinidir}/opentelemetry-python-contrib/opentelemetry-instrumentation {toxinidir}/opentelemetry-sdk {toxinidir}/tests/util

test-core-sdk: pip install {toxinidir}/opentelemetry-python-contrib/opentelemetry-instrumentation
test-core-opentracing-shim: pip install {toxinidir}/opentelemetry-python-contrib/opentelemetry-instrumentation

test-core-proto: pip install {toxinidir}/opentelemetry-proto
distro: pip install {toxinidir}/opentelemetry-python-contrib/opentelemetry-instrumentation {toxinidir}/opentelemetry-distro
Expand All @@ -135,7 +138,9 @@ commands_pre =
exporter-otlp-proto-grpc: pip install {toxinidir}/exporter/opentelemetry-exporter-otlp-proto-grpc

exporter-jaeger-combined: pip install {toxinidir}/exporter/opentelemetry-exporter-jaeger-proto-grpc {toxinidir}/exporter/opentelemetry-exporter-jaeger-thrift {toxinidir}/exporter/opentelemetry-exporter-jaeger
exporter-jaeger-combined: pip install {toxinidir}/opentelemetry-python-contrib/opentelemetry-instrumentation
exporter-jaeger-proto-grpc: pip install {toxinidir}/exporter/opentelemetry-exporter-jaeger-proto-grpc
exporter-jaeger-proto-grpc: pip install {toxinidir}/opentelemetry-python-contrib/opentelemetry-instrumentation
exporter-jaeger-thrift: pip install {toxinidir}/exporter/opentelemetry-exporter-jaeger-thrift

opentracing-shim: pip install {toxinidir}/opentelemetry-sdk
Expand Down Expand Up @@ -192,6 +197,7 @@ deps =
commands_pre =
python -m pip install -e {toxinidir}/opentelemetry-api[test]
python -m pip install -e {toxinidir}/opentelemetry-semantic-conventions[test]
python -m pip install {toxinidir}/opentelemetry-python-contrib/opentelemetry-instrumentation
python -m pip install -e {toxinidir}/opentelemetry-sdk[test]
python -m pip install -e {toxinidir}/opentelemetry-proto[test]
python -m pip install -e {toxinidir}/tests/util[test]
Expand All @@ -207,7 +213,6 @@ commands_pre =
python -m pip install -e {toxinidir}/exporter/opentelemetry-exporter-zipkin[test]
python -m pip install -e {toxinidir}/propagator/opentelemetry-propagator-b3[test]
python -m pip install -e {toxinidir}/propagator/opentelemetry-propagator-jaeger[test]
python -m pip install {toxinidir}/opentelemetry-python-contrib/opentelemetry-instrumentation
python -m pip install -e {toxinidir}/opentelemetry-distro[test]

commands =
Expand Down Expand Up @@ -239,8 +244,8 @@ deps =
commands_pre =
pip install -e {toxinidir}/opentelemetry-api \
-e {toxinidir}/opentelemetry-semantic-conventions \
-e {toxinidir}/opentelemetry-sdk \
-e {toxinidir}/opentelemetry-python-contrib/opentelemetry-instrumentation \
-e {toxinidir}/opentelemetry-sdk \
-e {toxinidir}/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-requests \
-e {toxinidir}/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-wsgi \
-e {toxinidir}/opentelemetry-python-contrib/util/opentelemetry-util-http
Expand Down