openzipkin-attic / zipkin-azure

Reporters and collectors for use in Azure
Apache License 2.0
11 stars 9 forks source link

Zipkin to Application Insights module #33

Open SergeyKanzhelev opened 7 years ago

SergeyKanzhelev commented 7 years ago

Issue to discuss ongoing work in PR: https://github.com/openzipkin/zipkin-azure/pull/27

The main idea is to allow to report and store Zipkin traces in the format Application Insights recognize so all the power of Application Insights Analytics engine, alerting and proactive detection can be used for those traces.

Also allows to view data collected by Application Insights SDK in Zipkin UI.

aliostad commented 7 years ago

@adriancole, @SergeyKanzhelev but I think we need some discussion over it.

I have some concerns over storing zipkin as ApplicationInsight. It would be unlikely that these format are like-for-like and need to understand of possible trade-offs that we might have to make as a result.

My view is that, doing the opposite ie making AI agent to send Zipkin traces, is a better option - I believe considering AI is a late-joiner to the game and people already use many different tools, it is ideal for AI to allow extension points to publish to other systems (zipkin, ELK, graphite, etc)

codefromthecrypt commented 7 years ago

@aliostad sorry my note was about sergey being active on many things tracing, and thanking for that engagement. that's what I meant by "apologies to watchers for using github comment as messaging I will delete this". Me thanking him for collaborating in general is independent of what direction is taken on this issue.

aliostad commented 7 years ago

@adriancole sure, sorry I think my comment probably came across differently :/ I thought you were about to delete this thread so asked if you do not and keep it open so we can discuss.

Thanks for all the hard work and sorry for the confusion.

aliostad commented 7 years ago

Just another note from my experience, we use a modified ELK stack (use conveyorbelt instead of logstash) and we will be independently writing to zipkin.

I think it is ambitious to try to bring AI and zipkin closely together and even if it is possible now, their independence might mean they will part ways further on so I prefer attacking the problem as an integration rather than composition.

codefromthecrypt commented 7 years ago

@adriancole https://github.com/adriancole sure, sorry I think my comment probably came across differently :/ I thought you were about to delete this thread so asked if you do not and keep it open so we can discuss.

Thanks for all the hard work and sorry for the confusion.

heh yeah ok. glad we're on-track again (sorry again for derailing)

SergeyKanzhelev commented 7 years ago

@aliostad, you are correct. For me one of the reason for this work is to understand how far apart Application Insights and Zipkin are with regards to data model. And we thinking of creating adapter that works backwards as well. Zipkin may be a perfect tool for the customers who cannot send data to the cloud or just want to use existing tools while using automatic data collection provided by Application Insights .NET SDK. However we still need to overcome the same gap between data models.

Once we will know the difference we can plan accordingly what changes can be done on our side and what we can contribute back. As you said we may never come to the exactly same schemas. And will always maintain the bridge between tools. However I only have the best intentions in mind and want to make it easy for developers to do distributed tracing.

@aliostad I believe Zipkin to Application Insights will be helpful for many people. If you have more concerns - please let me know.

In any case - it will be helpful if you can help with understanding of the data model gap, I was planning to post the list of questions we stumble across with @praveenbarli later today.

SergeyKanzhelev commented 7 years ago

So for the most part it's easy to match from Zipkin to Application Insights and back.

There is no concept of base span class. Application Insights defines request as incoming span and dependency as a span leaving process.

Every telemetry item can also have a set of context properties as well as generic name value dictionary.

Both request and dependency defines:

There are few pieces of information Application Insights typically collects that we cannot find a match for:

There are also a few context properties we typically collect. I didn't find matching concepts yet.

There are few more properties defined in Application Insights that we'd love to match, but they don't seems to exist as a strongly typed or well know concepts. I'm thinking there should be a configuration file to match binary annotations to well defined properties and back.

I keep trying different examples I can find to see how Zipkin is used. @adriancole do you know any examples of a good end-to-end demo app instrumented with Zipkin highlighting the best practices and possible scenarios?

codefromthecrypt commented 7 years ago

@adriancole https://github.com/adriancole do you know any examples of a good end-to-end demo app instrumented with Zipkin highlighting the best practices and possible scenarios?

There's no demo that shows all tracing use cases in the same app. There are a number of example apps, though:

https://github.com/criteo/zipkin4net/tree/master/zipkin4net-example https://github.com/openzipkin/brave-webmvc-example https://github.com/openzipkin/zipkin-js-example https://github.com/openzipkin/zipkin-go-opentracing/tree/master/examples

Also, at least in Brave, we have integration tests that show common instrumentation use cases: https://github.com/openzipkin/brave/blob/master/instrumentation/http-tests/src/main/java/brave/http/ITHttpClient.java https://github.com/openzipkin/brave/blob/master/instrumentation/http-tests/src/main/java/brave/http/ITHttpServer.java https://github.com/openzipkin/brave/blob/master/brave/src/test/java/brave/features/async/OneWaySpanTest.java

Finally, the thrift docs have comments defining a lot of the semantics in greater detail than things on http://zipkin.io https://github.com/openzipkin/zipkin-api/blob/master/thrift/zipkinCore.thrift

I suspect that if we needed to, we could make a super demo that included all integration patterns, just that hasn't been requested before.

SergeyKanzhelev commented 7 years ago

Thrift docs are very helpful! There are a lot of assumed semantic in annotations. How strict Zipkin collectors typically are? How many special cases there are in Zipkin UI for cases when one span has two CS annotations (or something like this)? If Application Insights storage implementation will reject bad formed spans - will it still cover majority of implementations or you'd recommend to not enforce anything?

codefromthecrypt commented 7 years ago

Thrift docs are very helpful! There are a lot of assumed semantic in annotations. How strict Zipkin collectors typically are? How many special cases there are in Zipkin UI for cases when one span has two CS annotations (or something like this)? If Application Insights storage implementation will reject bad formed spans - will it still cover majority of implementations or you'd recommend to not enforce anything?

Collectors will drop malformed (badly encoded data like required fields absent), but errors such as multiple "cs" are not dropped at the moment. Luckily these are indeed errors and instrumentation can be asked to fix them. Seeing that there were multiple "cs" sent help folks fix their instrumentation. Not sure if that answers your question.

SergeyKanzhelev commented 7 years ago

Zipkin 2 schema is very similar to Application Insights request and dependency data types. Interesting discussion there as well. It also refers to census spans converter: https://github.com/GoogleCloudPlatform/stackdriver-zipkin/blob/master/translation/src/main/java/com/google/cloud/trace/zipkin/translation/SpanTranslator.java Ideally Application Insights converter should use the same logic to detect the span kind as census uses

SergeyKanzhelev commented 7 years ago

@praveenbarli looking at how to map Application Insights data back to Zipkin span - came up with this query so far... Still thinking how to map the rest of possible properties:

//https://github.com/openzipkin/zipkin-api/blob/master/thrift/zipkinCore.thrift
requests 
| project 
    span_trace_id = operation_Id,
    span_name = name,
    span_id = id,
    span_parentId = operation_ParentId,
    span_timestamp = timestamp,
    span_duration = duration,
    annotations_sr_host_servcie_name = cloud_RoleName,
    annotations_ss_host_servcie_name = cloud_RoleName,
    binary_annotations_http_url = url,
    binary_annotations_http_status_code = resultCode,
    binary_annotations_ca_host_ip = client_IP,
    binary_annotations_ca_host_service_name = iff(isnull(source), operation_SyntheticSource, source),
    binary_annotations_extraStringValues = customDimensions,
    binary_annotations_extraDoubleValues = customMeasurements
    //success,
    //session_Id ,
    //user_Id ,
    //user_AuthenticatedId ,
    //user_AccountId ,
    //application_Version , 
    //client_Model ,
    //client_OS ,
    //client_Browser ,
    //cloud_RoleInstance ,
    //appName ,
    //sdkVersion ,
    //itemCount  
    // no intention to expose in Zipkin:
    //itemId ,
    //itemType ,
    //performanceBucket ,
    //operation_Name ,
    //client_Type ,
    //client_City ,
    //client_StateOrProvince ,
    //client_CountryOrRegion ,
    //appId ,
    //iKey
| take 10

You can use a similar query instead of ones you are using now.

praveenbarli commented 7 years ago

@SergeyKanzhelev sure. Once, I am able to write all these props I will start using this read query. One more question - I see in test cases writing Spans with neither cs/cr (dependencies) or sr/ss(requests). Which table would those go?

SergeyKanzhelev commented 7 years ago

@praveenbarli use dependency for the local ones.