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

samplingExcludedTypes is incorrect, it goes inside samplingSettings and is just called excludedTypes #47219

Closed
andrewleader opened this issue Jan 31, 2020 — with docs.microsoft.com · 3 comments

Comments

Copy link

The docs claim samplingExcludedTypes (and samplingIncludedTypes) are directly children of applicationInsights and NOT inside samplingSettings...

"applicationInsights": {
            "samplingSettings": {
              "isEnabled": true,
              "maxTelemetryItemsPerSecond" : 20,
              "evaluationInterval": "01:00:00",
              "initialSamplingPercentage": 1.0, 
              "samplingPercentageIncreaseTimeout" : "00:00:01",
              "samplingPercentageDecreaseTimeout" : "00:00:01",
              "minSamplingPercentage": 0.1,
              "maxSamplingPercentage": 0.1,
              "movingAverageRatio": 1.0
            },
            "samplingExcludedTypes" : "Dependency;Event",
            "samplingIncludedTypes" : "PageView;Trace",
            "enableLiveMetrics": true,

However, that's incorrect. It actually goes inside samplingSettings, and is called excludedTypes. I spent 40 minutes trying to figure out why the excluded types weren't working, all because the docs are wrong!

"applicationInsights": {
            "samplingSettings": {
              "isEnabled": true,
              "excludedTypes" : "Dependency;Event",
              "includedTypes" : "PageView;Trace",
            }
}

Their source code confirms this and was the reason I tried moving the property inside samplingSettings: https://github.com/Azure/azure-functions-host/blob/8e37c6f055d538460df6c18fc74c310edd58f225/src/WebJobs.Script/Config/ApplicationInsightsLoggerOptionsSetup.cs#L58


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

@DixitArora-MSFT
Copy link
Contributor

Hi @andrewleader Thanks for your feedback. We will review and update as appropriate.

@DixitArora-MSFT
Copy link
Contributor

@andrewleader PR has been created for this which will address this. Thanks so much for providing the feedback that help us in improving the documentation. For now I will proceed with closure of this and If there are further questions regarding this matter, please tag me in your reply. We will gladly continue the discussion and we will reopen the issue.

@andrewleader
Copy link
Author

@DixitArora-MSFT the property tables also need to be updated (that PR only updated the sample JSON, the rest of the doc needs to be updated)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants