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

SignalFx Naming Convention creates too many regex objects #3745

Closed
lenin-jaganathan opened this issue Apr 6, 2023 · 3 comments
Closed

SignalFx Naming Convention creates too many regex objects #3745

lenin-jaganathan opened this issue Apr 6, 2023 · 3 comments
Labels
performance Issues related to general performance registry: signalfx A SignalFX Registry related issue
Milestone

Comments

@lenin-jaganathan
Copy link
Contributor

Describe the bug
While exporting metrics (publish()), the SignalFx registry converts the tags into the SignalFx Dimension object. To stick to the SignalFx tag naming convention, it does a lot of the regex (5 to be precise) matching which created too much garbage objects.

Scenario: 10,000 metrics (Note: this is 10,000 metrics and not 10,000 meters. For, one timer we will have 4 metrics generated in case of SignalFx.) exported by application, with an average of 8 tags per metric.

For the above scenario, SignalFx creates the following

  • Create one Datapoint object for each metric (Since we have histograms as separate data points we end up creating 10,000 of these objects per minute)

  • Creates one Dimension Object per dimension per metric. (Total = 80,000 per minute)

  • 5 java.util.regex.Matcher objects per dimension. (Total = 400,000)

All the above are garbage collected.

Ref code:

public String tagKey(String key) {
String conventionKey = delegate.tagKey(key);
conventionKey = START_UNDERSCORE_PATTERN.matcher(conventionKey).replaceAll(""); // 2
conventionKey = SF_PATTERN.matcher(conventionKey).replaceAll(""); // 2
conventionKey = PATTERN_TAG_KEY_DENYLISTED_CHARS.matcher(conventionKey).replaceAll("_");
if (!START_LETTERS_PATTERN.matcher(conventionKey).matches()) { // 3
conventionKey = "a" + conventionKey;
}
if (PATTERN_TAG_KEY_DENYLISTED_PREFIX.matcher(conventionKey).matches()) {
String finalConventionKey = conventionKey;
logger.log(() -> "'" + finalConventionKey + "' (original name: '" + key + "') is not a valid tag key. "
+ "Must not start with any of these prefixes: aws_, gcp_, or azure_. "
+ "Please rename it to conform to the constraints. "
+ "If it comes from a third party, please use MeterFilter to rename it.");
}
return StringUtils.truncate(conventionKey, KEY_MAX_LENGTH); // 1
}

To Reproduce
Create a SignalFx Registry and register 10,000 meters with x tags each. Observe the memory increase right at the Step start time (when publishing happens). We will easily see (50,000 * x) number of regex objects created

Expected behavior
Garbage creation during publishing should be optimized to create as little as possible.

@lenin-jaganathan
Copy link
Contributor Author

I tried a trivial if else replacement for some of the regexes, which I am sharing in this Gist (https://gist.github.com/lenin-jaganathan/caf6e0d0ccf659a899846ee8012c5060). This helped reduce the garbage by up to 4x, on the average case (with 12k meters) we were seeing 175 Mb of garbage created which was reduced to around 65 Mb just with the above case.

@lenin-jaganathan
Copy link
Contributor Author

Another way of doing it is in the new meter () construct (paying the one-time cost), where we can validate the tags and add them with the right value to the meter itself. This way we can completely eliminate the re-construction of Objects during publish time (which happens very very frequently)

@lenin-jaganathan lenin-jaganathan changed the title SignalFx Naming Convention created too many regex objects SignalFx Naming Convention creates too many regex objects Apr 6, 2023
@shakuzen shakuzen added registry: signalfx A SignalFX Registry related issue waiting-for-triage labels Apr 7, 2023
@lenin-jaganathan
Copy link
Contributor Author

Well, using newMeter() construct will not be possible because of the implementation of registerMeterIfNecessary which checks if the meter exists. Will re-think about any better options.

@shakuzen shakuzen added performance Issues related to general performance and removed waiting-for-triage labels Apr 10, 2023
@shakuzen shakuzen added this to the 1.9.11 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Issues related to general performance registry: signalfx A SignalFX Registry related issue
Projects
None yet
Development

No branches or pull requests

2 participants