Adds environment variables for log exporter #3037

pridhi-arora · 2022-11-15T16:49:10Z

Description

Type of change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

tox -e test-exporter-otlp-proto-grpc
tox -e test-exporter-otlp-proto-http

Does This PR Require a Contrib Repo Change?

No.

Checklist:

Followed the style guidelines of this project
Unit tests have been added

pridhi-arora · 2022-11-15T16:50:07Z

@srikanthccv. This is the first draft of the PR. Please take a look.

srikanthccv · 2022-11-15T19:17:15Z

You are on the right track. Don't forget to add unit tests. Do you plan to implement the same for the HTTP exporter in another PR or update this PR itself?

pridhi-arora · 2022-11-16T04:34:12Z

I plan to add HTTP changes as well here. Do you want them to be a separate PR?

srikanthccv · 2022-11-16T10:34:57Z

It's up to you. I am fine with either way.

srikanthccv · 2022-12-03T12:25:40Z

Please fix the conflicts and mark it ready if the implementation is complete.

pridhi-arora · 2022-12-03T17:09:42Z

@srikanthccv Tests are not yet complete, I am trying to find the fix to the errors coming while writing the test.

pridhi-arora · 2022-12-06T15:19:44Z

@srikanthccv grpc tests are failing for me. HTTP one is passing.
Following is a small snippet of the error. I am trying to fix this.

logs/test_otlp_logs_exporter.py::TestOTLPLogExporter::test_env_variables FAILED                               [ 81%]
logs/test_otlp_logs_exporter.py::TestOTLPLogExporter::test_exporting FAILED                                   [ 83%]
logs/test_otlp_logs_exporter.py::TestOTLPLogExporter::test_failure FAILED                                     [ 84%]
logs/test_otlp_logs_exporter.py::TestOTLPLogExporter::test_no_credentials_error FAILED                        [ 86%]
logs/test_otlp_logs_exporter.py::TestOTLPLogExporter::test_otlp_exporter_endpoint FAILED                      [ 88%]
logs/test_otlp_logs_exporter.py::TestOTLPLogExporter::test_success FAILED                                     [ 89%]
logs/test_otlp_logs_exporter.py::TestOTLPLogExporter::test_translate_log_data FAILED                          [ 91%]
logs/test_otlp_logs_exporter.py::TestOTLPLogExporter::test_translate_multiple_logs FAILED                     [ 93%]
logs/test_otlp_logs_exporter.py::TestOTLPLogExporter::test_unavailable FAILED                                 [ 94%]
logs/test_otlp_logs_exporter.py::TestOTLPLogExporter::test_unavailable_delay FAILED                           [ 96%]
performance/benchmarks/test_benchmark_trace_exporter.py::test_simple_span_processor PASSED                    [ 98%]
performance/benchmarks/test_benchmark_trace_exporter.py::test_batch_span_processor
--------------------------------------------------- live log call ---------------------------------------------------
WARNING  opentelemetry.sdk.trace.export:__init__.py:234 Queue is full, likely spans will be dropped.
PASSED                                                                                                        [100%]

===================================================== FAILURES ======================================================
______________________________________ TestOTLPLogExporter.test_env_variables _______________________________________

self = <tests.logs.test_otlp_logs_exporter.TestOTLPLogExporter testMethod=test_env_variables>

    def setUp(self):

>       self.exporter = OTLPLogExporter()


logs/test_otlp_logs_exporter.py:115:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../../.tox/py310-opentelemetry-exporter-otlp-proto-grpc/lib/python3.10/site-packages/opentelemetry/exporter/otlp/proto/grpc/_log_exporter/__init__.py:93: in __init__
    super().__init__(
../../../.tox/py310-opentelemetry-exporter-otlp-proto-grpc/lib/python3.10/site-packages/opentelemetry/exporter/otlp/proto/grpc/exporter.py:231: in __init__
    parsed_url = urlparse(endpoint)
/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/urllib/parse.py:392: in urlparse
    url, scheme, _coerce_result = _coerce_args(url, scheme)
/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/urllib/parse.py:128: in _coerce_args
    return _decode_args(args) + (_encode_result,)
/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/urllib/parse.py:112: in _decode_args
    return tuple(x.decode(encoding, errors) if x else '' for x in args)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

.0 = <tuple_iterator object at 0x11186b6a0>

>   return tuple(x.decode(encoding, errors) if x else '' for x in args)
E   AttributeError: 'tuple' object has no attribute 'decode'

srikanthccv · 2022-12-07T17:41:25Z

Were you able to find out why?

pridhi-arora · 2022-12-08T02:03:01Z

@srikanthccv, I am not able to fix it. Can you help? Can you please point out what exactly am I doing wrong here, it will unblock me.

srikanthccv · 2022-12-08T04:50:54Z

When you have any failures, please try to understand the message and trace it back. It says AttributeError: 'tuple' object has no attribute 'decode' . The first thing to notice here is why is decode called on a tuple object? Are we passing an incorrect type somewhere? And it also shows the stack trace with line numbers where the error originated.

.../python3.10/site-packages/opentelemetry/exporter/otlp/proto/grpc/exporter.py:231: in __init__
    parsed_url = urlparse(endpoint)

It says line 231 in exporter.py is the reason which links to one more invocation up in this call stack.

.../python3.10/site-packages/opentelemetry/exporter/otlp/proto/grpc/_log_exporter/__init__.py:93: in __init__
    super().__init__(

This is where it all started, line 93 in grpc/_log_exporter/__init__.py; at this point, you can check what types of all the args passed from super().__init_are_

There are two lines in you code which I believe are the reason.

...
        endpoint=endpoint or environ.get(OTEL_EXPORTER_OTLP_LOGS_ENDPOINT),

        headers=headers or environ.get(OTEL_EXPORTER_OTLP_LOGS_HEADERS),
...

I hope you know what trailing comma (,) in Python does

>>>
>>> foo = 1,
>>> foo
(1,)

pridhi-arora · 2022-12-08T06:16:28Z

@srikanthccv, I get it now. So, in layman's terms, the "," was making the variable a tuple and the __init__.py was not expecting a tuple, hence the error.
I have fixed that issue, but now. I am facing another one.
The mock_exporter_mixin is coming out empty for me. I tried to debug it using the print statement. I observe that it is empty. debugging tests

This is the output I am getting.

*************************
[]

srikanthccv · 2022-12-08T11:27:41Z

Please spend some time reading about how mocks work and when the args list is non-empty.

pridhi-arora · 2022-12-12T10:02:32Z

@srikanthccv, I read mocking and tried to fix the error. I had to call,

exporter = OTLPLogExporter()
for this particular test so that the env variables get injected into the newly created object.
https://github.com/open-telemetry/opentelemetry-python/pull/3037/files#diff-e111afdd5be7166307664a5c9924d73217142dafc9d816428b3ffd152ce4de64R115

The tests are still failing for me. Trying to find the fix.

pridhi-arora · 2022-12-12T11:06:36Z

I tried to look for all the properties the exporter is exposing via exporter.__dict__

I only get the following.

def test_env_variables(self):
        exporter = OTLPLogExporter()
        print("**************")
        print(exporter._headers)
        print("*******endpoint*******")
        print(exporter.__dict__)

        # print(exporter.)
>       self.assertEqual(exporter._endpoint, "logs:4317")
E       AttributeError: 'OTLPLogExporter' object has no attribute '_endpoint'

logs/test_otlp_logs_exporter.py:198: AttributeError
--------------------------------------------------------- Captured stdout call ---------------------------------------------------------
**************
(('key1', 'value1'), ('key2', 'VALUE=2'), ('user-agent', 'OTel OTLP Exporter Python/1.16.0.dev'))
*******endpoint*******
{'_headers': (('key1', 'value1'), ('key2', 'VALUE=2'), ('user-agent', 'OTel OTLP Exporter Python/1.16.0.dev')), '_timeout': 10, '_collector_kwargs': None, '_client': <opentelemetry.proto.collector.logs.v1.logs_service_pb2_grpc.LogsServiceStub object at 0x1081d4eb0>}

How do I test properties like
endpoint, _compression and _credentials,

Ref: 9447d0e#diff-b46561f17e94b2de768ab7d81658ef144122a638a7ffb3fcd82fe122a9561b37R194-R195

pridhi-arora · 2022-12-15T09:54:35Z

@srikanthccv Do I have to add the label to "skip public api check symbol"?

srikanthccv · 2023-01-09T19:26:03Z

@pridhi-arora One last change before we merge this. Please update the minimum required SDK version to 1.16.0.dev here and here because the exporter now depend on the newly added env in SDK.

srikanthccv · 2023-01-11T12:46:40Z

@pridhi-arora friendly ping

Signed-off-by: PridhiArora <pridhiarora17@gmail.com>

pridhi-arora · 2023-01-11T15:28:17Z

@srikanthccv Done!

srikanthccv · 2023-01-11T17:28:39Z

Thanks!

* main: fixed all instances of @tracer.start_as_current_span("name"): to @tracer.start_as_current_span("name") as decorators do not have colons (open-telemetry#3127) Add attribute name to type warning message. (open-telemetry#3124) Fix requirements file for example (open-telemetry#3126) Add db metric name to semantic conventions (open-telemetry#3115) Adds environment variables for log exporter (open-telemetry#3037) Fix bug in example (open-telemetry#3111)

pridhi-arora force-pushed the logenv branch from 48e766b to a81fdf5 Compare November 16, 2022 05:00

pridhi-arora force-pushed the logenv branch from 1d78d88 to fc6b263 Compare December 6, 2022 11:07

pridhi-arora force-pushed the logenv branch from b6277e9 to 55c17e9 Compare December 8, 2022 06:12

pridhi-arora added 4 commits December 12, 2022 15:26

Adds environment variables for log exporter

ab5f6a0

Add tests for env variables introduced in logger.

3ddc60d

Debugging tests

fccf547

Tried to fix tests

b17d41f

pridhi-arora force-pushed the logenv branch from c1c06e2 to b17d41f Compare December 12, 2022 09:57

Finding out properties of log exporter

9447d0e

Fixed the logger tests finally

e10fed6

pridhi-arora marked this pull request as ready for review December 15, 2022 08:38

pridhi-arora requested a review from a team December 15, 2022 08:38

pridhi-arora added 2 commits December 15, 2022 15:16

Fixes lint

7307117

Update ChangeLog

66a0787

Updated http log env tests

dbfa4c2

Merge branch 'main' into logenv

17aab8d

srikanthccv added the Approve Public API check This label shows that the public symbols added or changed in a PR are strictly necessary label Jan 3, 2023

Merge branch 'main' into logenv

a22506d

lzchen approved these changes Jan 9, 2023

View reviewed changes

Merge branch 'main' into logenv

d6748e7

Updated opetelemetry sdk version

d8f2405

Signed-off-by: PridhiArora <pridhiarora17@gmail.com>

srikanthccv approved these changes Jan 11, 2023

View reviewed changes

srikanthccv merged commit 665bd48 into open-telemetry:main Jan 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds environment variables for log exporter #3037

Adds environment variables for log exporter #3037

pridhi-arora commented Nov 15, 2022 •

edited by srikanthccv

Loading

pridhi-arora commented Nov 15, 2022

srikanthccv commented Nov 15, 2022

pridhi-arora commented Nov 16, 2022

srikanthccv commented Nov 16, 2022

srikanthccv commented Dec 3, 2022

pridhi-arora commented Dec 3, 2022

pridhi-arora commented Dec 6, 2022 •

edited

Loading

srikanthccv commented Dec 7, 2022

pridhi-arora commented Dec 8, 2022

srikanthccv commented Dec 8, 2022

pridhi-arora commented Dec 8, 2022 •

edited

Loading

srikanthccv commented Dec 8, 2022

pridhi-arora commented Dec 12, 2022

pridhi-arora commented Dec 12, 2022 •

edited

Loading

pridhi-arora commented Dec 15, 2022

srikanthccv commented Jan 9, 2023

srikanthccv commented Jan 11, 2023

pridhi-arora commented Jan 11, 2023

srikanthccv commented Jan 11, 2023

Adds environment variables for log exporter #3037

Adds environment variables for log exporter #3037

Conversation

pridhi-arora commented Nov 15, 2022 • edited by srikanthccv Loading

Description

Type of change

How Has This Been Tested?

Does This PR Require a Contrib Repo Change?

Checklist:

pridhi-arora commented Nov 15, 2022

srikanthccv commented Nov 15, 2022

pridhi-arora commented Nov 16, 2022

srikanthccv commented Nov 16, 2022

srikanthccv commented Dec 3, 2022

pridhi-arora commented Dec 3, 2022

pridhi-arora commented Dec 6, 2022 • edited Loading

srikanthccv commented Dec 7, 2022

pridhi-arora commented Dec 8, 2022

srikanthccv commented Dec 8, 2022

pridhi-arora commented Dec 8, 2022 • edited Loading

srikanthccv commented Dec 8, 2022

pridhi-arora commented Dec 12, 2022

pridhi-arora commented Dec 12, 2022 • edited Loading

pridhi-arora commented Dec 15, 2022

srikanthccv commented Jan 9, 2023

srikanthccv commented Jan 11, 2023

pridhi-arora commented Jan 11, 2023

srikanthccv commented Jan 11, 2023

pridhi-arora commented Nov 15, 2022 •

edited by srikanthccv

Loading

pridhi-arora commented Dec 6, 2022 •

edited

Loading

pridhi-arora commented Dec 8, 2022 •

edited

Loading

pridhi-arora commented Dec 12, 2022 •

edited

Loading