-
Notifications
You must be signed in to change notification settings - Fork 98
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.
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
Add documentation on installation and usage #132
Add documentation on installation and usage #132
Conversation
docs/USAGE.md
Outdated
| `OTEL_ENV` | The value for the `environment` tag added to every span. | | | ||
| `OTEL_TRACE_ENABLED` | Enable to activate the tracer. | `true` | | ||
| `OTEL_TRACE_DEBUG` | Enable to activate debugging mode for the tracer. | `false` | | ||
| `OTEL_TRACE_AGENT_URL` | The URL to where send the traces. | `http://localhost:9080/v1/trace` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the default dependent on the OTel exporter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well it seems it's not, at least not in this env variable, but this is still incorrect. It should be http://localhost:8126
following this:
var agentUri = source?.GetString(ConfigurationKeys.AgentUri) ?? // default value $"http://{agentHost}:{agentPort}";
if the agnetHost and agentPort are not set in env variables then it takes constant defaults: localhost
and 8126
for port.
Jaeger uses its own configuration key (OTEL_EXPORTER_JAEGER_AGENT_HOST
). Zipkin and default one use OTEL_TRACE_AGENT_URL
. I don't know if it should work that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to fix Zipkin to also use its own env var. Now that config sources are available that shouldn't be a problem. Could you please open an issue for that?
| `OTEL_DISABLED_INTEGRATIONS` | The integrations you want to disable, if any, separated by a semi-colon. These are the supported integrations: AspNetMvc, AspNetWebApi2, DbCommand, ElasticsearchNet5, ElasticsearchNet6, GraphQL, HttpMessageHandler, IDbCommand, MongoDb, NpgsqlCommand, OpenTracing, ServiceStackRedis, SqlCommand, StackExchangeRedis, Wcf, WebRequest | | | ||
| `OTEL_CONVENTION` | Sets the outbound http and trace id convention for tracer. Available values are: `Datadog` (64bit trace id), `OpenTelemetry` (128 bit trace id). | `Datadog` | | ||
| `OTEL_PROPAGATORS` | Semicollon-separated list of the propagators for tracer. Available propagators are: `Datadog`, `B3`, `W3C`. The Tracer will try to execute extraction in the passed order. | `Datadog` | | ||
| `OTEL_TRACE_DOMAIN_NEUTRAL_INSTRUMENTATION` | Sets whether to intercept method calls when the caller method is inside a domain-neutral assembly. This is recommended when instrumenting IIS applications. | `false` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this env var exists anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is same as here. It is there in cpp code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment on the cpp code it is pretty good: "environment variable is not needed when running on .NET Framework 4.5.2 or higher, and will be ignored.". Adding something like: "only needed on .NET Framework prior to version 4.5.2" seems a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor things to clear
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add something like:
## Usage
See [USAGE.md](USAGE.md) for installation, usage and configuration instructions.
to the mail README.md
?
docs/USAGE.md
Outdated
Choose the installer (x64 or x86) according to the architecture of the application | ||
you're instrumenting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose the installer (x64 or x86) according to the architecture of the application | |
you're instrumenting. | |
Choose the installer (x64 or x86) according to the architecture of the operating | |
system where it will be running. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because the x64 MSI will install both the 32-bit and 64-bit profiler onto the machine. The x86 MSI will only install the 32-bit profiler onto the machine, since it can only run 32-bit processes.
What
The USAGE.md was added to the docs directory. It includes installation steps for autoinstrumentation as well as env variables for configuration.
Why
For now this repository has no information about the installation, configuration or usage. This PR should resolve Issue#111.