diff --git a/plugins/experimental/rate_limit/limiter.h b/plugins/experimental/rate_limit/limiter.h index 038c295faf1..715cb6316e6 100644 --- a/plugins/experimental/rate_limit/limiter.h +++ b/plugins/experimental/rate_limit/limiter.h @@ -224,8 +224,8 @@ template class RateLimiter std::string prefix = metrics["prefix"] ? metrics["prefix"].as() : RATE_LIMITER_METRIC_PREFIX; std::string tag = metrics["tag"] ? metrics["tag"].as() : name(); - Dbg(dbg_ctl, "Metrics for selector rule: %s(%s, %s)", name().c_str(), prefix.c_str(), tag.c_str()); - initializeMetrics(RATE_LIMITER_TYPE_SNI, prefix, tag); + Dbg(dbg_ctl, "Metrics for selector rule: %s (prefix=%s, tag=%s)", name().c_str(), prefix.c_str(), tag.c_str()); + initializeMetrics(RATE_LIMITER_TYPE_SNI, tag, prefix); } return true; diff --git a/tests/gold_tests/pluginTest/rate_limit/rate_limit_metric_names.test.py b/tests/gold_tests/pluginTest/rate_limit/rate_limit_metric_names.test.py new file mode 100644 index 00000000000..85a8fca820e --- /dev/null +++ b/tests/gold_tests/pluginTest/rate_limit/rate_limit_metric_names.test.py @@ -0,0 +1,97 @@ +''' +Test that the rate_limit YAML metrics node names metrics as prefix.type.tag. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +Test.Summary = ''' +rate_limit: the YAML metrics node builds names as prefix.type.tag. +''' + +Test.SkipUnless(Condition.PluginExists('rate_limit.so')) +Test.ContinueOnFail = True + +server = Test.MakeOriginServer("server") +server.addResponse( + "sessionlog.json", { + "headers": "GET /health HTTP/1.1\r\nHost: metrics.example.com\r\n\r\n", + "timestamp": "1469733493.993", + "body": "" + }, { + "headers": "HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\n", + "timestamp": "1469733493.993", + "body": "OK" + }) + +ts = Test.MakeATSProcess("ts") + +# Three selectors: both keys set, only the tag set, and only the prefix set. +# The defaults are the metric prefix and the SNI respectively, so a swap is +# visible in every one of them. +rate_limit_yaml = os.path.join(ts.Variables.CONFIGDIR, 'rate_limit.yaml') +ts.Disk.File( + rate_limit_yaml, typename="ats:config").AddLines( + [ + 'selector:', + ' - sni: both.example.com', + ' limit: 100', + ' metrics:', + ' prefix: myprefix', + ' tag: mytag', + ' - sni: tagonly.example.com', + ' limit: 100', + ' metrics:', + ' tag: onlytag', + ' - sni: prefixonly.example.com', + ' limit: 100', + ' metrics:', + ' prefix: onlyprefix', + '', + ]) + +ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'rate_limit', +}) + +ts.Disk.remap_config.AddLine(f'map / http://127.0.0.1:{server.Variables.Port}/') +ts.Disk.plugin_config.AddLine(f'rate_limit.so {rate_limit_yaml}') + +# The metrics are registered by TSStatCreate while the config is parsed, so they +# are queryable as soon as ATS is up, with no traffic needed. +tr = Test.AddTestRun("Metric names use the configured prefix and tag") +tr.Processes.Default.StartBefore(server, ready=When.PortOpen(server.Variables.Port)) +tr.Processes.Default.StartBefore(ts) +tr.Processes.Default.Command = 'traffic_ctl metric match sni' +tr.Processes.Default.ReturnCode = 0 +tr.Processes.Default.Env = ts.Env +tr.StillRunningAfter = ts + +tr.Processes.Default.Streams.All = Testers.ContainsExpression( + 'myprefix.sni.mytag.queued', 'prefix and tag should keep their configured positions') +tr.Processes.Default.Streams.All += Testers.ExcludesExpression('mytag.sni.myprefix', 'the prefix and tag must not be swapped') + +tr.Processes.Default.Streams.All += Testers.ContainsExpression( + 'plugin.rate_limiter.sni.onlytag.queued', 'an unset prefix should fall back to the default prefix') +tr.Processes.Default.Streams.All += Testers.ExcludesExpression( + 'onlytag.sni.plugin.rate_limiter', 'the default prefix must not be used as the tag') + +tr.Processes.Default.Streams.All += Testers.ContainsExpression( + 'onlyprefix.sni.prefixonly.example.com.queued', 'an unset tag should fall back to the SNI') +tr.Processes.Default.Streams.All += Testers.ExcludesExpression( + 'prefixonly.example.com.sni.onlyprefix', 'the SNI must not be used as the prefix')