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.

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

Refine setup and configuration docs #2082

Merged
merged 5 commits into from
Jan 27, 2023

Conversation

pellared
Copy link
Member

@pellared pellared commented Jan 26, 2023

Why

Fixes #2071

Towards #2076

What

  • Add recommendation for configuration methods
  • Update IIS instrumentation docs
  • Update Windows Service instrumentation docs
  • Add comments for Bash script
  • Update PowerShell module docs

@github-actions github-actions bot requested a review from theletterf January 26, 2023 15:50
@pellared
Copy link
Member Author

@ramanjaneyagupta PTAL and feel free to provide any feedback

docs/README.md Outdated
# Get current installation location
Get-OpenTelemetryInstallDirectory
# Run your application with instrumentation
dotnet run
Copy link
Member Author

@pellared pellared Jan 26, 2023

Choose a reason for hiding this comment

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

I know that dotnet run is not currently working. Reference #1744

@pjanotti Should we keep it or change the example to use some executable? E.g.

Suggested change
dotnet run
.\MyNetApp.exe

Copy link
Contributor

Choose a reason for hiding this comment

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

I will explicitly call out the current issues with dotnet CLI and suggest to create a shell to run the instrumented version. Basically: build and run dotnet CLI on shell 1 then set up the environment for automatic instrumentation on shell 2. A warning icon, plus, a link to the bug seems worthwhile at the moment.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will address it in a separate PR.

@pellared pellared marked this pull request as ready for review January 26, 2023 16:01
@pellared pellared requested a review from a team January 26, 2023 16:01
@pellared
Copy link
Member Author

@theletterf PTAL. Lots of docs changes

Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

@pellared for the time being we need to ensure that the instrumentation environment variables are not set in the same context/shell in which dotnet CLI is being used. There are too many setting combinations and most of them lead to different problems. So explicitly call that out and recommend running the executable directly in a separate context/shell to get instrumentation going.

docs/README.md Outdated
# Get current installation location
Get-OpenTelemetryInstallDirectory
# Run your application with instrumentation
dotnet run
Copy link
Contributor

Choose a reason for hiding this comment

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

I will explicitly call out the current issues with dotnet CLI and suggest to create a shell to run the instrumented version. Basically: build and run dotnet CLI on shell 1 then set up the environment for automatic instrumentation on shell 2. A warning icon, plus, a link to the bug seems worthwhile at the moment.

docs/README.md Outdated Show resolved Hide resolved
docs/iis-instrumentation.md Outdated Show resolved Hide resolved
docs/iis-instrumentation.md Outdated Show resolved Hide resolved
docs/windows-service-instrumentation.md Outdated Show resolved Hide resolved
docs/windows-service-instrumentation.md Outdated Show resolved Hide resolved
docs/windows-service-instrumentation.md Outdated Show resolved Hide resolved
docs/windows-service-instrumentation.md Outdated Show resolved Hide resolved
Kielek and others added 2 commits January 27, 2023 06:48
@pellared pellared merged commit d9169af into open-telemetry:main Jan 27, 2023
@pellared pellared deleted the config-recommendation branch January 27, 2023 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document recommendations regarding configuration methods
4 participants