This repository has been archived by the owner on Aug 22, 2022. It is now read-only.
[SE-3136] Add client instance name to New Relic monitoring condition name #640
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ndition name To be able to find root cause of incidents faster and improve the investigation experience, adding the client's instance name to the monitoring condition's name. In case the name is too long, it will be truncated due to limitations.
…me is shortened if needed
|
@toxinu Added an extra test to ensure that the condition name is not exceeding the (assumed) maximum length of the field. |
toxinu
approved these changes
Sep 13, 2020
gabor-boros
added a commit
that referenced
this pull request
Sep 25, 2020
…name (#640) * feat(monitoring): Add client instance name to New Relic monitoring condition name To be able to find root cause of incidents faster and improve the investigation experience, adding the client's instance name to the monitoring condition's name. In case the name is too long, it will be truncated due to limitations. * test(monitoring): Add extra test case to make sure that monitoring name is shortened if needed * style: Reformat the new unit test to match the quality standards
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is adding client instance name to New Relic monitoring condition name to be able to find root cause of incidents faster and improve the investigation experience. In case the name is too long, it will be truncated due to limitations.
Dependencies: None
Screenshots:
Merge deadline: None
Testing instructions:
ADMINSsetting set in your local.envfile, set it to your email (ie.ADMINS='[["Gabor Admin", "gabor@opencraft.com"]]'.NEWRELIC_ADMIN_USER_API_KEYandNEWRELIC_LICENSE_KEYin your local.envfile. You can find them on the production Ocim VM's.envfile.make shell).OpenEdXInstace(or create a new one if you don't have an existing instance in your devstack).instance.enable_monitoring(). Make sure that there are no errors.instance.disable_monitoring().NEWRELIC_ADMIN_USER_API_KEYandNEWRELIC_LICENSE_KEYfrom your local.envfile!Author notes and concerns:
.... Example faulty condition name:Preview of ...[1] [2][1] In my opinion, this shouldn't be an issue since it would mean that the instance name should be at least 42 characters long without any whitespace. Number 42 is built up from: length of the longest "prefix" with formatting extracted from 64, the current maximum length. So:
64 - len("Extended heartbeat of ")[2] The 64 length is not confirmed by NewRelic developers yet. Currently, I'm investigating the max length with the help of New Relic, but I think, this shouldn't be a blocker, since an institution name which is equal to or longer than 42 chars is really rare.
UPDATE: 64 chars max length is confirmed by New Relic