census-instrumentation / opencensus-csharp Goto Github PK
View Code? Open in Web Editor NEWDistributed tracing and stats collecting framework
Home Page: https://opencensus.io
License: Apache License 2.0
Distributed tracing and stats collecting framework
Home Page: https://opencensus.io
License: Apache License 2.0
C# and .NET programming styles do not typically involve interface-for-everything approach.
We should have a discussion about it in opencensus and during separation of API and Impl (#8) ensure that interfaces are not overused
Motivation: Performance
User impact: None/Hypothetical
Oops, that wasn't meant to make it in as-is, mostly just to demonstrate the problem with the test :)
RE: #29, a lock is being taken for every read. We should investigate alternative methods as the type has the concurrency properties of being unlikely to be written to frequently (in fact, it's disallowed) and being likely to be read more than it is written. Perhaps ReaderWriterLock would be sufficient here.
Can impl package depend on Ms.Extensions.DependencyInjection? It should be just fine for .NET Core apps.
In this case TraceComponent could be removed completely
Could they be removed at least from API package?
This repository uses pattern matching with some errors:
The purpose of Match
method is to avoid if/else
checks by supplying set of callbacks to call. And each implementation calls respective callback. Something like a virtual methods in generic class, but strongly typed for pre-defined types.
Issues:
AttributeValue
uses if/else instead of having separate classes with the hardcoded implementation of a Match
method which will call the proper callback:
There is no need to expose type-specific results in factory methods AttributeValue.StringAttributeValue
. Simply returning IAttributeValue
should suffice. So generic IAttributeValue<T>
interface is not needed.
Implementation of IAttributeValue is trivial and there is no need to have interface and implementation separately - simple implementation class will suffice.
In the java source version, the concrete classes are created by AutoValue which, from my limited understanding, does not expose the concrete types. If we want to preserve that intention, then I'd propose our concrete classes (like BucketBoundaries
, should not be sealed
, but rather be static
with just the Create
method, and have private sealed
implementation classes.
If the intention is to expose these concrete types (which may result in folks programming to those rather than the intended interfaces) then that's fine too - but at that point we may want to consider if we want to expose interfaces at all.
Support statsZ page
This is the issue to collect feedback on releases schedule for this repository. Current thinking is to schedule releases like one beta every month and stable version twice a year. Basically calendar-based, not feature based.
We will also align marking SDK 1.0 with other repositories.
Currently opencensus targets 4.6 and netstandard 2.0.
.NET 4.5 is not officially supported by Microsoft, but still is very popular.
Caveat: AsyncLocal
is not available on .NET 4.5.
Here is the code that System.Diagnostics.Activity
uses instead. Note what it does to avoid cross AppDomain calls issues.
See motivation for Activity: https://github.com/dotnet/corefx/issues/29207
TL;DR;
There are certain cases when async context is not reliable. The most popular case is native code invocation which is common case in ASP.NET classic or libs like System.Drawing.
We should be able to provide users and integrations a way to restore context.
Motivation: Exposed API
User impact: Confusion
The Value
property on an instance of CurrentStatsState
exposes an empty set
method. Either
Set(StatsCollectionState state)
We should have license for all classes. See example in other repos.
Eliminate this hardcode in auto-collector:
I suggest as an initial implementation just have a custom sampler that will work per-request object before the main sampler like suggested here: https://github.com/rakyll/opencensus-specs/blob/7ae3810847d951ae79d0576afc3fd2cf0e020862/trace/HTTP.md#sampling
I am assuming this should work but it doesnt listen on anything related to port 9184 then, if I change it to localhost
instead of 0.0.0.0
then it works. Is it a bug or intentional?
var exporter = new PrometheusExporter(
new PrometheusExporterOptions()
{
Url = new Uri("http://0.0.0.0:9184/metrics/")
},
Stats.ViewManager);
exporter.Start();
All exporter are expected to send data asynchronously, this method must be async
Currently we don't have metrics and resource specifications implemented:
https://github.com/census-instrumentation/opencensus-specs/blob/master/metrics/Metrics.md
https://github.com/census-instrumentation/opencensus-proto/blob/master/src/opencensus/proto/metrics/v1/metrics.proto
we can leverage all existing instrumentation with DiagnosticSource and auto-collect Asp.NET Core, Http, Sql, ServiceBus, EventHubs
In some environments opening separate port for Prometheus exporter will not work. Especially for sandboxed environments like Azure Web Apps. Re-using app's port and allow Prometheus to work as a middleware will help to enable those environments.
Hi, is there a timeline for support for EF core and psql/mysql/mssql?
The spec statement about implementing the Context concept is
Languages that already have this support, like Go (context.Context) or C# (ExecutionContext), MUST use the language supported generic context instead of building their own.
Is there more info about a planned design in this direction? ExecutionContext doesn't seem great for extension, but I might be missing something.
https://docs.microsoft.com/en-us/dotnet/api/system.threading.executioncontext?view=netcore-2.0
So that library can take a dependency on API only allowing multiple implementations and more flexibility on impl side
We typically log all internal errors/warning/traces to EventSource in other .NET libs.
Let's try to start with CircleCI to align with other repos.
Guid.NextGuid().GetBytes
is a better alternative
Hi,
Thank you for working on this project. I tried to get latest beta build from MyGet today (i am particularly interested in #109) and realized that CI is red from the end of January.
Failing test is OpenCensus.Exporter.ApplicationInsights.Tests.OpenCensusTelemetryConverterTests.OpenCensusTelemetryConverterTests_TracksRequestWithAnnotations
which fails on Assert.NonEqual
. I do not have enough knowledge to understand why it fails.
Thanks.
'monitorring' is the tag instead of 'monitoring'
Implement exporter to local forwarder (grpc, using https://github.com/census-instrumentation/opencensus-proto/blob/master/opencensus/proto/exporter/exporter.proto)
Hi!
Is Jaeger exporter considered?
Reviewing API surface - IList is used when IEnumerable will suffice. For instance in Exporter handler and other places. IList allows manipulating elements - add, remove, etc. when this is absolutely not the intent.
Check ocagent exporter: it has to convert any attribute to string because there is no way to check value type:
Check how it's done for protobuf oneof case: https://github.com/Microsoft/ApplicationInsights-LocalForwarder/blob/master/src/Library/OpenCensusTelemetryConverter.cs#L527
Support RPCz page to expose span buckets
Span.Kind
should be set early to allow correct metrics calculation. This should be reflected in API design - getter/setter pattern may not be ideal here. Consider requiring span Kind
in constructor or Create
method.
tracestate may carry sampling-related information. So ShouldSample method of ISampler must take it as an argument
Clean up time-related contracts. Most of them came as a port from Java SDK. After reviewing it I think we need the following:
Timestamp
class with nanosecond resolution. Make it struct. It should only used in SpanData
as this resolution it only needed to be able to represent high-resolution clock - when you are importing data for instance, not to actually measure time in C# application. It may also be needed for stats Views
, need more investigations.IClock
should be removed as it is only used to abstract away getNanos
system call in Java. The only promise of IClock
is to give the start time of a span and DateTimeOffset.Now
can do it well.TimesampConverter
is needed as it is used to calculate latency with high precision using StopWatch
from the startTime
of the outermost span. All children spans and time events should the same instance of TimestampConverter
to ensure the durations and position of time events is monotonic and precise. Theoretically we can use Stopwatch
class instead of using this wrapper. The only benefit of a wrapper is that it will encapsulate both - start time and stopwatch to generate high-precision start time.Mostly for @lmolkova (I think). I found this doc while looking for something else https://docs.microsoft.com/en-us/dotnet/framework/debug-trace-profile/tracing-and-instrumenting-applications and it looks like it's saying for dotnet to use Trace
and doesn't mention Activity
. There may be reasons for this I just wanted to put it on your radar.
Code from SteelToe will be ported to this repository: https://github.com/SteeltoeOSS/Management/tree/dev/src/Steeltoe.Management.OpenCensus
You can start looking into code and plan for contributions
hi, there.
I'm trying to use your library. Thank you for it.
So, I found some inconsistency in the implementation of exporters.
Namely, 'Sum' aggregation in case of Prometheus converted to 'gauge', in case of StackDriver to CUMULATIVE, which is 'counter'.
The fun is that neither of both is totally right.
'Sum' can represent either 'counter' or 'gauge'. For instance, when we count peers or users currently online it is very convenient to use 'Sum' and, in this case, it should be converted to 'gauge' by all exporters because value can go up and down
The second case, when we count the amount of data sent. When we have just buffer length it is very convenient to use 'Sum', but in this case, a value will only grow and measure should be converted to 'counter'
So, I would propose to add a second type for 'Sum' aggregation. first will be converted to 'gauge' second to 'counter'
ByteToHexCharArray seems to be extremely inefficient with all allocations it does. Here is a wonderful perf analysis comparison for different algos to convert byte array to string https://stackoverflow.com/questions/311165/how-do-you-convert-a-byte-array-to-a-hexadecimal-string-and-vice-versa
we should do this: https://stackoverflow.com/questions/311165/how-do-you-convert-a-byte-array-to-a-hexadecimal-string-and-vice-versa/24343727#24343727
The proposal is to implement an optional out parameter in StartScopedSpan
API so you can use using {}
pattern and do not need to get the current span inside the using
block again. Other scenarios performance may also be improved
subj
There are only a few aggregation types currently supported by Prometheus exporter. Need to finalize all of them.
See
for list of TODOsAs described in #92 - there are cases when you might need to report a span from the past. In that PR Redis library calls are recorded asynchronously. My guess why this API was chosen to expose Redis calls is extreme care of those calls latency.
Current API is quite limiting in this sense.
ISpan
doesn't allow to set start and end timestamps explicitly. Same for time events like annotations. Expanding ISpan
interface may create some confusions.SpanData
directly. This approach can be problematic as a lot of logic like calling start/stop handler and sampling needs to be replicated manually.ISpan
or make ISpan
constructible from SpanData
. I like this last approach. Not sure what may be the problems here.I've tried using distribution aggregation with stackdriver exporter, but it does not work.
It looks like the same issue as in GO package - census-instrumentation/opencensus-go#143
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.