Skip to content

[OTel] unittest coverage improvement#603

Closed
CagriYonca wants to merge 172 commits into
otel_migrationfrom
otel/unittest_coverage
Closed

[OTel] unittest coverage improvement#603
CagriYonca wants to merge 172 commits into
otel_migrationfrom
otel/unittest_coverage

Conversation

@CagriYonca

@CagriYonca CagriYonca commented Aug 29, 2024

Copy link
Copy Markdown
Contributor

I worked on the modules below, increased existing unittests and created non-existing:

  • src/instana/collector/base.py (100% coverage)
  • src/instana/collector/helpers/runtime.py (97%)
  • src/instana/collector/host.py (96%)
  • src/instana/collector/utils.py (100%)
  • src/instana/propagators/base_propagator.py (96%)
  • src/instana/sampling.py (100%)
  • src/instana/util/traceutils.py (100%)

@CagriYonca CagriYonca added this to the H2-2024 milestone Aug 29, 2024
@CagriYonca CagriYonca requested review from GSVarsha and pvital August 29, 2024 17:32
@CagriYonca CagriYonca self-assigned this Aug 29, 2024
@CagriYonca CagriYonca added the OTel_migration Migration the code dependency from OpenTracing to OpenTelemetry label Aug 30, 2024
@CagriYonca CagriYonca force-pushed the otel/unittest_coverage branch 4 times, most recently from f5d8227 to 80ddc10 Compare September 4, 2024 08:38
Comment thread src/instana/propagators/base_propagator.py Outdated
@CagriYonca CagriYonca force-pushed the otel/unittest_coverage branch 2 times, most recently from 43dead3 to 9bd8989 Compare September 5, 2024 09:24
@CagriYonca CagriYonca requested a review from GSVarsha September 5, 2024 09:24
pvital and others added 15 commits September 12, 2024 15:10
setup.py configuration to replace the dependency of OpenTracing with OpenTelemetry (OTel) and bump up to version 3.0.0.

Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
Disabled all tests that are not necessary in the beginning of the migration.

Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
- Implement all the abstract methods provided by the OTel API
- Adapt the existing code to OTel conventions.
- Inherit OTel's SpanContext.
- Comment out the usage of baggage.
- Use time in nano seconds for start_time and end_time.
- Initialization of Span's attributes, events, start_time, status, duration,
and synthetic.
- Add the duration, status, and parent_id properties.
- Minor fixes to set values or get a dictionary value.

Co-authored-by: Paulo Vital <paulo.vital@ibm.com>
Signed-off-by: Varsha GS <varsha.gs@ibm.com>
Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
…olations.

Used ruff (vscode) to:
- Black-compatible code formatting.
- fix all auto-fixable violations, like unused imports.
- isort-compatible import sorting.

Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
Co-authored-by: Varsha GS <varsha.gs@ibm.com>
Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
Used ruff (vscode) to:
- Black-compatible code formatting.
- fix all auto-fixable violations, like unused imports.
- isort-compatible import sorting.

Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
Signed-off-by: Varsha GS <varsha.gs@ibm.com>
- contextmanager for start_as_current_span().
- use trace_flags.sampled instead of sampled.
- Add Instana specific attributes to SpanContext.
- Add parent Span_Context class arguments as necessary arguments to fix
the serialization of binary objects.
- Use format_span_id() for both representation of the trace_id and span_id
since Instana uses 64-bit integers.

Co-authored-by: Paulo Vital <paulo.vital@ibm.com>
Signed-off-by: Varsha GS <varsha.gs@ibm.com>
- Refactor InstanaTracerProvider class for better performance.
- Add the missing add_span_processor() method for InstanaTracerProvider.
- Refactor InstanaTracer constructor removing all Optional arguments.
- Fix the start_span() method to handle the root SpanContext properly.
- Use TraceFlags as trace_flags arguments in new SpanContext objects.

Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
Following the OpenTelemetry API, guarantee Traces and Spans IDs are now
64-bit integers instead of strings. The data transmited to the Instana
Agents or Backend are still 16HEXDIG strings.

Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
Disabled the load of the auto instrumentation for tests and the load
of the instana package from the beginning of test execution.

Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
Adds a pytest.mark.skipif running on macOS to avoid the raise of a
NotImplementedError when calling multiprocessing.Queue.qsize().

Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
@CagriYonca CagriYonca force-pushed the otel/unittest_coverage branch 2 times, most recently from ca256ee to 27b3398 Compare September 18, 2024 09:31
Comment thread src/instana/collector/base.py Outdated
Comment thread src/instana/span_context.py Outdated
Comment thread src/instana/span.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This file is outdated. Please take a look at src/instana/span folder if you want to make some changes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👆🏻 this!

@GSVarsha GSVarsha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Few requests ⬆️

CagriYonca and others added 5 commits September 19, 2024 09:55
Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
Remove the TestAgent and the if-blocks that check if running in a test
environment from the production code.

Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
@CagriYonca CagriYonca force-pushed the otel/unittest_coverage branch from 27b3398 to e50ea19 Compare September 20, 2024 14:06

@pvital pvital left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Few requests

Comment thread src/instana/collector/base.py Outdated
Comment thread src/instana/span.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👆🏻 this!

Comment thread tests/collector/__init__.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You are moving/renaming the __init__.py file from the old platforms directory to the collector, and letting the new agents directory without an init file.

I suggest you to:

  • First, rename the new agents dir name to agent - coherent with the same name in src/instana/
  • Second, create a new __init__.py file in the tests/collector folder and move this one to the new agent dir.

Comment thread tests/util/test_traceutils.py Outdated
Comment on lines 15 to 18

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need to have this if you are not setting up and tearing down the test execution.

Comment thread tests/util/test_traceutils.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Missing type annotation.

CagriYonca and others added 9 commits September 20, 2024 17:30
Signed-off-by: Varsha GS <varsha.gs@ibm.com>
(cherry picked from commit 8ab3210a92502253876f91213009c287167f6790)
(cherry picked from commit 126e4ec09440509da867e7b7aa49dfaf0c8a867d)
Signed-off-by: Varsha GS <varsha.gs@ibm.com>
Signed-off-by: Varsha GS <varsha.gs@ibm.com>
Signed-off-by: Varsha GS <varsha.gs@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement OTel_migration Migration the code dependency from OpenTracing to OpenTelemetry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants