[monitor] README + samples _ ci.yml #15288

rakshith91 · 2020-11-12T18:45:57Z

No description provided.

hectorhdzg · 2020-11-12T18:50:14Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

Please add gitter link

Actually I believe there is a gitter for Azure SDK, we can deprecate the old one and use the SDK one now
https://gitter.im/Azure/azure-sdk-for-python

hectorhdzg · 2020-11-12T18:52:47Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

This could confuse users here, need to add details about logging of the actual library, sounds like a magic flag that will enable sending logging telemetry

i believe this should actually be removed for now

hectorhdzg · 2020-11-12T18:56:17Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

Both examples you have in here are actually using BatchExportSpanProcessor, this is example is about using instrumentors, we need extra details like original readme to make clear what the example is covering
https://github.com/microsoft/opentelemetry-azure-monitor-python#instrumentations

hectorhdzg · 2020-11-12T18:57:03Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

Modifying traces is a bid deal for our customers, please add the example as well
https://github.com/microsoft/opentelemetry-azure-monitor-python#modifying-traces

hectorhdzg · 2020-11-12T18:59:37Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

What about the auto generated documentation? similar to https://azuresdkdocs.blob.core.windows.net/$web/python/azure-mgmt-monitor/1.0.1/index.html

We do have the API ref docs link right on top of the page. The intention of theseadditional docs are more to give an in depth idea of the service, ot etc.

scbedd · 2020-11-12T19:07:05Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

telemetry

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/samples/README.md

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

sdk/monitor/ci.yml

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

lzchen · 2020-11-12T22:29:59Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

If we are moving towards the use of connection string, would it be better to omit anything related to instrumentation key and only recommend connection string?

I agree with that tbh! There should be one - and preferably only one obvious way to do it :)
Let's stop documenting it in the readme for this beta but still support it, that way we dont break much stuff on the day of the release but will not encourage users to use it.

lzchen · 2020-11-12T22:31:28Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

I believe the original sdks used APPLICATIONINSIGHTS_CONNECTION_STRING and APPINSIGHTS_INSTRUMENTATIONKEY?

yup, we changed these in the samples, so just being consistent

Wouldn't this be a breaking change if user's are instrumented with both old SDKs and OT sdk? That means they would have to have 2 environment variables to represent the same thing.

fair enough - changed it

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

lzchen · 2020-11-12T22:46:25Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

I would link to the code in which defines "Data"

lzchen · 2020-11-12T22:47:29Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

Isn't this step already explained above?

agreed - this has become redundant

lzchen · 2020-11-12T22:48:09Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

Don't think the exporter README should contain anything about setting up the App Insights resource.

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

lzchen · 2020-11-12T22:48:45Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

Is requests_ linking to anything?

lzchen · 2020-11-12T22:50:12Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

The telemetry processor should not be in the simple example.

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

lzchen · 2020-11-12T22:52:23Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

I would comment a comment for this line saying something like: This line causes your calls made with the requests library to be tracked.

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/samples/traces/README.md

…les/README.md

Co-authored-by: Scott Beddall <[email protected]>

…ME.md Co-authored-by: Leighton Chen <[email protected]>

Co-authored-by: Leighton Chen <[email protected]>

…ME.md

lzchen · 2020-11-13T19:02:20Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

+### Prerequisites: 
+To use this package, you must have:
+* Azure subscription - [Create a free account][azure_sub]
+* Azure Monitor - [How to use application insights][application_insights_namespace]


Is this really a prerequisite? Also Azure Monitor - How to use Application Insights is kind of confusing. Shouldn't we be only introducing Azure Monitor?

lzchen · 2020-11-13T19:11:20Z

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md

+
+* [Opentelemetry][opentelemtry_spec]: Opentelemetry is a set of libraries used to collect and export telemetry data (metrics, logs, and traces) for analysis in order to understand your software's performance and behavior.
+
+* [Instrumentation][instrumentation_library]: The ability to call the opentelemetry API directly by any application is facilitated by instrumentaton. A library that enables OpenTelemetry observability for another library is called an Instrumentation Library.


Suggested change

* [Instrumentation][instrumentation_library]: The ability to call the opentelemetry API directly by any application is facilitated by instrumentaton. A library that enables OpenTelemetry observability for another library is called an Instrumentation Library.

* [Instrumentation][instrumentation_library]: The ability to track calls made by a Python library is facilitated by instrumentations. A library that enables OpenTelemetry observability for one of these Python libraries is called an Instrumentation Library.

I would also link to the supported instrumentation libraries that Python supports. https://github.com/open-telemetry/opentelemetry-python-contrib/tree/master/instrumentation

…into new_polling * 'master' of https://github.com/Azure/azure-sdk-for-python: (281 commits) [Communication] Add variable groups necessary for using new azure subscription (Azure#15213) Sync eng/common directory with azure-sdk-tools for PR 1153 (Azure#15322) [Communication] Update version for Preview 3 release (Azure#15317) Increment package version after release of azure_data_tables (Azure#15291) [monitor] README + samples _ ci.yml (Azure#15288) refdoc aka.ms link was broken and silently redirecting, have replaced with azuresdkdocs link as is used in other SDKs. (Azure#15169) Sync eng/common directory with azure-sdk-tools for PR 1202 (Azure#15297) Updating OT exporter to consume latest version of OpenTelemetry API/SDK (Azure#15289) Change live test resource DeleteAfterHours tag to 8 hours (Azure#15294) Add Invoke-DevOpsAPI.ps1, Add functions for Canceling and Listing Builds (Azure#15013) The original one is removed. Point to the new one (Azure#15287) Sync eng/common directory with azure-sdk-tools for PR 1170 (Azure#15087) Adding OpenTelemetry exporter code (Azure#14784) update external link (Azure#15273) Remove invalid characters in basename sourced from username (Azure#15268) Sync eng/common directory with azure-sdk-tools for PR 1188 (Azure#15267) Increment version for core releases (Azure#15269) Increment package version after release of azure_identity (Azure#15266) Fix CI (Azure#15265) Increment package version after release of azure_eventgrid (Azure#15262) ...

rakshith91 requested review from danieljurek, lmazuel, mayurid and mitchdenny as code owners November 12, 2020 18:45

hectorhdzg reviewed Nov 12, 2020

View reviewed changes

scbedd reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated

Copy link

Member

scbedd Nov 12, 2020

Choose a reason for hiding this comment

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

telemetry

rakshith91 reacted with laugh emoji

rakshith91 commented Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/samples/README.md Outdated Show resolved Hide resolved

hectorhdzg previously approved these changes Nov 12, 2020

View reviewed changes

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

rakshith91 dismissed hectorhdzg’s stale review via 1ad1ecd November 12, 2020 20:29

scbedd reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/ci.yml Outdated Show resolved Hide resolved

rakshith91 commented Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated

Copy link

Member

lzchen Nov 12, 2020

Choose a reason for hiding this comment

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

I would link to the code in which defines "Data"

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/README.md Outdated Show resolved Hide resolved

lzchen reviewed Nov 12, 2020

View reviewed changes

sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/samples/traces/README.md Outdated Show resolved Hide resolved

rakshith91 and others added 8 commits November 13, 2020 16:33

README + minor improvs

9353d91

changes

f0555ab

Update sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/samp…

5a60629

…les/README.md

samples readme

87c8211

Update sdk/monitor/ci.yml

354dbc0

Co-authored-by: Scott Beddall <[email protected]>

Update sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/READ…

9d591b6

…ME.md Co-authored-by: Leighton Chen <[email protected]>

Update sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/READ…

6f8796c

…ME.md Co-authored-by: Leighton Chen <[email protected]>

readme

e58a295

rakshith91 force-pushed the otexpreadme branch from 3598e7a to e58a295 Compare November 13, 2020 11:03

Rakshith Bhyravabhotla and others added 4 commits November 13, 2020 16:36

Apply suggestions from code review

d639018

Co-authored-by: Leighton Chen <[email protected]>

Update sdk/monitor/microsoft-opentelemetry-exporter-azuremonitor/READ…

b75fd50

…ME.md

comments

84ad95b

conn str

2f53478

lzchen reviewed Nov 13, 2020

View reviewed changes

lzchen approved these changes Nov 13, 2020

View reviewed changes

rakshith91 merged commit 969b0ea into Azure:master Nov 13, 2020


		* [Opentelemetry][opentelemtry_spec]: Opentelemetry is a set of libraries used to collect and export telemetry data (metrics, logs, and traces) for analysis in order to understand your software's performance and behavior.

		* [Instrumentation][instrumentation_library]: The ability to call the opentelemetry API directly by any application is facilitated by instrumentaton. A library that enables OpenTelemetry observability for another library is called an Instrumentation Library.

[monitor] README + samples _ ci.yml #15288

[monitor] README + samples _ ci.yml #15288

Uh oh!

Conversation

rakshith91 commented Nov 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rakshith91 Nov 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

rakshith91 Nov 12, 2020 •

edited

Loading