-
Notifications
You must be signed in to change notification settings - Fork 64
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
Setting from NR backend should override setting from MAX_EVENT_SAMPLES_STORED and MAX_TRANSACTION_SAMPLES_STORED env var #1239
Conversation
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'd like to add some clarifying comments to help the next person to look at this understand the precedence order of ServerOverrides vs. EnvironmentOverrides and why it is different from other cases.
…on.cs Co-authored-by: Alex Hemsath <57361211+nr-ahemsath@users.noreply.github.com>
…on.cs Co-authored-by: Alex Hemsath <57361211+nr-ahemsath@users.noreply.github.com>
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.
Looks good!
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.
Looks even better than before!
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.
@vuqtran88 Josh reminded me that we need a changelog entry for this
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.
Thanks for updating the changelog. 💯
Description
At the current state, when MAX_EVENT_SAMPLES_STORED env var is set, the agent uses value from the env var for the preconnect request. The server replies with a smaller value (whatver the value sent up get divided by 12). However, the agent won't use the value from the server because our current logic allows the env to override any config values. This causes configuration mismatched as well as incorrect
...\EvenHarvest\CustomEventData\HarvestLimit
supportability metric value being reported.The same story happens to the MAX_TRANSACTION_SAMPLES_STORED env var usage.
The right approach is to let the server config to dictate the final value that the agent will use.