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.

Sign up for GitHub

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

Jump to bottom

Skeleton for azure monitor exporters #151

Merged
merged 10 commits into from
Sep 25, 2019
Merged

Skeleton for azure monitor exporters #151

merged 10 commits into from
Sep 25, 2019

Conversation

reyang
Copy link
Member

@reyang reyang commented Sep 19, 2019 •
edited
Loading

No description provided.

@mauriciovasquezbernal
Copy link
Member

Would it make sense to create a silly console exporter that could also work as a skeleton for more sophisticated implementations? I mean, just like this one but totally independent from Azure and that we will keep on the repo for reference purposes.

@reyang
Copy link
Member Author

reyang commented Sep 19, 2019 •
edited
Loading

Would it make sense to create a silly console exporter that could also work as a skeleton for more sophisticated implementations? I mean, just like this one but totally independent from Azure and that we will keep on the repo for reference purposes.

I think this is a good idea! I can add it in this PR.

Update: discussed in the community meeting, we will use sync console exporter, and put it inside the SDK instead of having a separate package.
Will have a separate package demonstrating how to implement an exporter package as part of the example project.

@reyang reyang changed the title Skeleton for azure monitor exporters Console exporter + skeleton for azure monitor exporters Sep 19, 2019
lzchen
lzchen approved these changes Sep 19, 2019
View reviewed changes
Copy link
Contributor

@lzchen lzchen left a comment

Choose a reason for hiding this comment

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

LGTM

@Oberon00
Copy link
Member

Ideally, this PR should be split up (the Span __repr__ has nothing to do with the title).

mauriciovasquezbernal
mauriciovasquezbernal reviewed Sep 20, 2019
View reviewed changes
opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py Outdated
@@ -280,7 +280,21 @@ def __init__(
self.start_time = None

def __repr__(self):
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this long version be __str__? I mean, to keep __repr__ as it is and add this new as __str__.
Maybe https://stackoverflow.com/questions/1436703/difference-between-str-and-repr could clarify things up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point.
Addressed in #156.

@mauriciovasquezbernal
Copy link
Member

Maybe the azure related changes could be added as a new PR?, so by just looking at that PR people can understand what is the exact code needed for a skeleton.

@reyang
Copy link
Member Author

reyang commented Sep 20, 2019

I've put another PR #156 which only contains the Span __str__ and __repr__ and console exporter.
Once #156 gets merged, will rebase this one.

@reyang reyang changed the title Console exporter + skeleton for azure monitor exporters Skeleton for azure monitor exporters Sep 20, 2019
@lzchen
Copy link
Contributor

lzchen commented Sep 21, 2019

Reviewed again after rebase. LGTM.

mauriciovasquezbernal
mauriciovasquezbernal approved these changes Sep 23, 2019
View reviewed changes
Copy link
Member

@mauriciovasquezbernal mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

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

LGMT!

c24t
c24t approved these changes Sep 24, 2019
View reviewed changes
Copy link
Member

@c24t c24t left a comment

Choose a reason for hiding this comment

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

LGTM after fixing some copypasta

ext/opentelemetry-ext-azure-monitor/setup.cfg Outdated Show resolved Hide resolved
@reyang @c24t
Update ext/opentelemetry-ext-azure-monitor/setup.cfg
c90d3f2 
Co-Authored-By: Chris Kleinknecht <libc@google.com>
@reyang reyang merged commit b657c27 into master Sep 25, 2019
@reyang reyang deleted the azure branch September 25, 2019 03:13
hectorhdzg added a commit to hectorhdzg/opentelemetry-python that referenced this pull request Oct 21, 2019
@hectorhdzg
Span add override parameters
c432da3 
for start_time and end_time

Make lint happy

Addressing comments

Addressing comments

Allowing 0 as start and end time

Fix lint issues

Metrics API RFC 0003 cont'd (open-telemetry#136)

* Create functions

Comments for Meter

More comments

Add more comments

Fix typos

* fix lint

* Fix lint

* fix typing

* Remove options, constructors, seperate labels

* Consistent naming for float and int

* Abstract time series

* Use ABC

* Fix typo

* Fix docs

* seperate measure classes

* Add examples

* fix lint

* Update to RFC 0003

* Add spancontext, measurebatch

* Fix docs

* Fix comments

* fix lint

* fix lint

* fix lint

* skip examples

* white space

* fix spacing

* fix imports

* fix imports

* LabelValues to str

* Black formatting

* fix isort

* Remove aggregation

* Fix names

* Remove aggregation from docs

* Fix lint

* metric changes

* Typing

* Fix lint

* Fix lint

* Add space

* Fix lint

* fix comments

* address comments

* fix comments

Adding a working propagator, adding to integrations and example (open-telemetry#137)

Adding a full, end-to-end example of propagation at work in the
example application, including a test.

Adding the use of propagators into the integrations.

Metrics API RFC 0009 (open-telemetry#140)

* Create functions

Comments for Meter

More comments

Add more comments

Fix typos

* fix lint

* Fix lint

* fix typing

* Remove options, constructors, seperate labels

* Consistent naming for float and int

* Abstract time series

* Use ABC

* Fix typo

* Fix docs

* seperate measure classes

* Add examples

* fix lint

* Update to RFC 0003

* Add spancontext, measurebatch

* Fix docs

* Fix comments

* fix lint

* fix lint

* fix lint

* skip examples

* white space

* fix spacing

* fix imports

* fix imports

* LabelValues to str

* Black formatting

* fix isort

* Remove aggregation

* Fix names

* Remove aggregation from docs

* Fix lint

* metric changes

* Typing

* Fix lint

* Fix lint

* Add space

* Fix lint

* fix comments

* handle, recordbatch

* docs

* Update recordbatch

* black

* Fix typo

* remove ValueType

* fix lint

Console exporter (open-telemetry#156)

Make use_span more flexible (closes open-telemetry#147). (open-telemetry#154)

Co-Authored-By: Reiley Yang <reyang@microsoft.com>
Co-Authored-By: Chris Kleinknecht <libc@google.com>

WSGI fixes (open-telemetry#148)

Fix http.url.
Don't delay calling wrapped app.

Skeleton for azure monitor exporters (open-telemetry#151)

Add link to docs to README (open-telemetry#170)

Move example app to the examples folder (open-telemetry#172)

WSGI: Fix port 80 always appended in http.host (open-telemetry#173)

Build and host docs via github action (open-telemetry#167)

Add missing license boilerplate to a few files (open-telemetry#176)

sdk/trace/exporters: add batch span processor exporter (open-telemetry#153)

The exporters specification states that two built-in span processors should be
implemented, the simple processor span and the batch processor span.

This commit implements the latter, it is mainly based on the opentelemetry/java
one.

The algorithm implements the following logic:
- a condition variable is used to notify the worker thread in case the queue
is half full, so that exporting can start before the queue gets full and spans
are dropped.
- export is called each schedule_delay_millis if there is a least one new span
to export.
- when the processor is shutdown all remaining spans are exported.

Implementing W3C TraceContext (fixes open-telemetry#116) (open-telemetry#180)

* Implementing TraceContext (fixes open-telemetry#116)

This introduces a w3c TraceContext propagator, primarily inspired by opencensus.

fix time conversion bug (open-telemetry#182)

Introduce Context.suppress_instrumentation (open-telemetry#181)

Metrics Implementation (open-telemetry#160)

* Create functions

Comments for Meter

More comments

Add more comments

Fix typos

* fix lint

* Fix lint

* fix typing

* Remove options, constructors, seperate labels

* Consistent naming for float and int

* Abstract time series

* Use ABC

* Fix typo

* Fix docs

* seperate measure classes

* Add examples

* fix lint

* Update to RFC 0003

* Add spancontext, measurebatch

* Fix docs

* Fix comments

* fix lint

* fix lint

* fix lint

* skip examples

* white space

* fix spacing

* fix imports

* fix imports

* LabelValues to str

* Black formatting

* fix isort

* Remove aggregation

* Fix names

* Remove aggregation from docs

* Fix lint

* metric changes

* Typing

* Fix lint

* Fix lint

* Add space

* Fix lint

* fix comments

* handle, recordbatch

* docs

* Update recordbatch

* black

* Fix typo

* remove ValueType

* fix lint

* sdk

* metrics

* example

* counter

* Tests

* Address comments

* ADd tests

* Fix typing and examples

* black

* fix lint

* remove override

* Fix tests

* mypy

* fix lint

* fix type

* fix typing

* fix tests

* isort

* isort

* isort

* isort

* noop

* lint

* lint

* fix tuple typing

* fix type

* black

* address comments

* fix type

* fix lint

* remove imports

* default tests

* fix lint

* usse sequence

* remove ellipses

* remove ellipses

* black

* Fix typo

* fix example

* fix type

* fix type

* address comments

Implement Azure Monitor Exporter (open-telemetry#175)

Span add override parameters for start_time and end_time (open-telemetry#179)

CONTRIBUTING.md: Fix clone URL (open-telemetry#177)

Add B3 exporter to alpha release table (open-telemetry#164)

Update README for alpha release (open-telemetry#189)

Update Contributing.md doc (open-telemetry#194)

Add **simple** client/server examples (open-telemetry#191)

Remove unused dev-requirements.txt (open-telemetry#200)

The requirements are contained in tox.ini now.

Fx bug in BoundedList for Python 3.4 and add tests (open-telemetry#199)

* fix bug in BoundedList for python 3.4 and add tests

collections.deque.copy() was introduced in python 3.5, this commit changes
that by the deque constructor and adds some tests to BoundedList and BoundedDict
to avoid similar problems in the future.

Also, improve docstrings of BoundedList and BoundedDict classes

Move util.time_ns to API. (open-telemetry#205)

Add Jaeger exporter (open-telemetry#174)

This adds a Jeager exporter for OpenTelemetry.  This exporter is based
on https://github.com/census-instrumentation/opencensus-python/tree/master/contrib/opencensus-ext-jaeger.

The exporter uses thrift and can be configured to send data to the agent and
also to a remote collector.

There is a long discussion going on about how to include generated files
in the repo, so for now just put them here.

Add code coverage

Revert latest commit

Fix some "errors" found by mypy. (open-telemetry#204)

Fix some errors found by mypy (split from open-telemetry#201).

Update README for new milestones (open-telemetry#218)

Refactor current span handling for newly created spans. (open-telemetry#198)

1. Make Tracer.start_span() simply create and start the Span,
   without setting it as the current instance.
2. Add an extra Tracer.start_as_current_span() to create the
   Span and set it as the current instance automatically.

Co-Authored-By: Chris Kleinknecht <libc@google.com>

Add set_status to Span (open-telemetry#213)

Initial commit

Initial version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@c24t c24t c24t approved these changes

@mauriciovasquezbernal mauriciovasquezbernal mauriciovasquezbernal approved these changes

@lzchen lzchen lzchen approved these changes

@a-feld a-feld Awaiting requested review from a-feld

@carlosalberto carlosalberto Awaiting requested review from carlosalberto

@Oberon00 Oberon00 Awaiting requested review from Oberon00

@toumorokoshi toumorokoshi Awaiting requested review from toumorokoshi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@reyang @mauriciovasquezbernal @Oberon00 @lzchen @c24t