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

Fix to run the Auto instrumentation example in the docs #1435

Merged
merged 24 commits into from
Dec 8, 2020
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
c8580b2
Merge pull request #1 from open-telemetry/master
dmarar Nov 27, 2020
207502b
Fix to run the auto-instrumentation example successfully
dmarar Nov 29, 2020
144e85c
reverting the changes in ConsoleSpanExporter, setup.cfg
dmarar Nov 29, 2020
18f3375
Merge pull request #2 from open-telemetry/master
dmarar Nov 30, 2020
5db7f6c
Merge pull request #3 from dmarar/master
dmarar Nov 30, 2020
ffb7ae3
changes to include a consolespanexporter and consolemetricsexporter a…
dmarar Nov 30, 2020
f60c0be
Change as per review comments to replace **kwargs with service_name
dmarar Dec 1, 2020
7af07ba
moving the service_name as the first parameter in the ConsoleSpanExpo…
dmarar Dec 1, 2020
8039aeb
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
dmarar Dec 3, 2020
374bec9
Merge branch 'master' into auto-instru-example-fix
lzchen Dec 3, 2020
88926f5
changes to add the service_name to the exported span
dmarar Dec 3, 2020
acd9ad1
Merge branch 'master' into auto-instru-example-fix
lzchen Dec 3, 2020
698b6fb
Merge branch 'master' into auto-instru-example-fix
lzchen Dec 4, 2020
6063a13
pass the service name to the exported span
dmarar Dec 4, 2020
a85f9f1
Merge pull request #4 from dmarar/new_test
dmarar Dec 4, 2020
6c47b67
update the setup.cfg to the correct path of ConsoleMetricsExporter
dmarar Dec 4, 2020
aa41b03
Merge pull request #5 from dmarar/new_test
dmarar Dec 4, 2020
3b0e945
Merge pull request #6 from open-telemetry/master
dmarar Dec 4, 2020
715c08e
Merge pull request #7 from open-telemetry/master
dmarar Dec 7, 2020
2ed540e
Merge pull request #8 from dmarar/master
dmarar Dec 7, 2020
6619a37
revert the changes to add the service_name to the exported span.
dmarar Dec 7, 2020
62d7ed0
Merge pull request #9 from open-telemetry/master
dmarar Dec 8, 2020
9482378
Merge branch 'master' into auto-instru-example-fix
lzchen Dec 8, 2020
6496413
Merge branch 'master' into auto-instru-example-fix
lzchen Dec 8, 2020
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
22 changes: 16 additions & 6 deletions docs/examples/auto-instrumentation/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,8 @@ Manually instrumented server
def server_request():
with tracer.start_as_current_span(
"server_request",
parent=propagators.extract(
lambda dict_, key: dict_.get(key, []), request.headers
)["current-span"],
context=propagators.extract(DictGetter(), request.headers
),
):
print(request.args.get("param"))
return "served"
Expand Down Expand Up @@ -137,7 +136,12 @@ similar to the following example:
"http.flavor": "1.1"
},
"events": [],
"links": []
"links": [],
"resource": {
"telemetry.sdk.language": "python",
"telemetry.sdk.name": "opentelemetry",
"telemetry.sdk.version": "0.16b1"
}
}

Execute an automatically instrumented server
Expand All @@ -148,7 +152,7 @@ and run the following command instead:

.. code:: sh

$ opentelemetry-instrument python server_uninstrumented.py
$ opentelemetry-instrument -e console_span,console_metrics python server_uninstrumented.py

In the console where you previously executed ``client.py``, run the following
command again:
Expand Down Expand Up @@ -192,7 +196,13 @@ similar to the following example:
"http.status_code": 200
},
"events": [],
"links": []
"links": [],
"resource": {
"telemetry.sdk.language": "python",
"telemetry.sdk.name": "opentelemetry",
"telemetry.sdk.version": "0.16b1",
"service.name": ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"service.name": ""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lzchen , this change is not done by me. it comes from the init_tracing code in components.py :

def init_tracing(
    exporters: Sequence[SpanExporter], ids_generator: trace.IdsGenerator
):
    service_name = get_service_name()
    provider = TracerProvider(
        resource=Resource.create({"service.name": service_name}),
        ids_generator=ids_generator(),
    )
    trace.set_tracer_provider(provider)

Copy link
Contributor

Choose a reason for hiding this comment

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

The outputs should be the same for auto-instrumented and manually instrumented. Since you removed service_name from appearing in the console export, it shouldn't appear for either example.

Copy link
Contributor Author

@dmarar dmarar Dec 8, 2020

Choose a reason for hiding this comment

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

@lzchen , there was mistake in that output snapshot in previous comment, the service.name was coming irrespective of my change.

The change I made adds an additional "service_name" to the output (please see the output) ( which i removed as per your suggestion)
auto-instru-example1

The code in the init_tracing hasnt been touched by me for any of the changes I did.

My apologies that i realised it very late that output in the previous comment was incorrect.

Please note that "service.name" is not added in the server output if server_instrument.py is run.

}
}

You can see that both outputs are the same because automatic instrumentation does
Expand Down
5 changes: 2 additions & 3 deletions docs/examples/auto-instrumentation/server_instrumented.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
ConsoleSpanExporter,
SimpleExportSpanProcessor,
)
from opentelemetry.trace.propagation.textmap import DictGetter

app = Flask(__name__)

Expand All @@ -36,9 +37,7 @@
def server_request():
with tracer.start_as_current_span(
"server_request",
parent=propagators.extract(
lambda dict_, key: dict_.get(key, []), request.headers
)["current-span"],
context=propagators.extract(DictGetter(), request.headers),
kind=trace.SpanKind.SERVER,
attributes=collect_request_attributes(request.environ),
):
Expand Down
3 changes: 3 additions & 0 deletions opentelemetry-sdk/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ opentelemetry_tracer_provider =
sdk_tracer_provider = opentelemetry.sdk.trace:TracerProvider
opentelemetry_propagator =
b3 = opentelemetry.sdk.trace.propagation.b3_format:B3Format
opentelemetry_exporter =
console_span = opentelemetry.sdk.trace.export:ConsoleSpanExporter
console_metrics = opentelemetry.sdk.metrics.export:ConsoleMetricsExporter

[options.extras_require]
test =
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@
# limitations under the License.

import collections
import json
import logging
import os
import sys
import threading
import typing
from enum import Enum
from typing import Optional

from opentelemetry.configuration import Configuration
from opentelemetry.context import Context, attach, detach, set_value
Expand Down Expand Up @@ -370,12 +372,14 @@ class ConsoleSpanExporter(SpanExporter):

def __init__(
self,
service_name: Optional[str] = None,
out: typing.IO = sys.stdout,
formatter: typing.Callable[[Span], str] = lambda span: span.to_json()
+ os.linesep,
):
self.out = out
self.formatter = formatter
self.service_name = service_name

def export(self, spans: typing.Sequence[Span]) -> SpanExportResult:
dmarar marked this conversation as resolved.
Show resolved Hide resolved
for span in spans:
Expand Down
3 changes: 2 additions & 1 deletion opentelemetry-sdk/tests/trace/export/test_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -477,14 +477,15 @@ def test_batch_span_processor_parameters(self):
class TestConsoleSpanExporter(unittest.TestCase):
def test_export(self): # pylint: disable=no-self-use
"""Check that the console exporter prints spans."""
exporter = export.ConsoleSpanExporter()

exporter = export.ConsoleSpanExporter()
# Mocking stdout interferes with debugging and test reporting, mock on
# the exporter instance instead.
span = trace._Span("span name", trace_api.INVALID_SPAN_CONTEXT)
with mock.patch.object(exporter, "out") as mock_stdout:
exporter.export([span])
mock_stdout.write.assert_called_once_with(span.to_json() + os.linesep)

self.assertEqual(mock_stdout.write.call_count, 1)
self.assertEqual(mock_stdout.flush.call_count, 1)

Expand Down