nickva commented on code in PR #4474:
URL: https://github.com/apache/couchdb/pull/4474#discussion_r1134512851


##########
src/couch_prometheus/src/couch_prometheus_util.erl:
##########
@@ -146,9 +146,9 @@ path_to_name(Path) ->
 
 counter_metric(Path) ->
     Name = path_to_name(Path),
-    case string:find(Name, <<"_total">>, trailing) == <<"_total">> of
-        true -> Name;
-        false -> to_bin(io_lib:format("~s_total", [Name]))
+    case string:find(Name, <<"_total">>, trailing) of
+        nomatch -> to_bin(io_lib:format("~s_total", [Name]));
+        _ -> Name

Review Comment:
   Do we care about a suffix only? Otherwise `string:find/3` will find 
non-suffix `_total` strings as well.
   
   ```erlang
   string:find("my_total_metric", <<"_total">>, trailing).
   "_total_metric"
   ```
   
   The issue previously was likely the list vs binary confusion. `string` 
functions will return either one depending on input. To avoid any confusion a 
simple `lists:suffix/2` might work better?
   
   ```erlang
   > true = lists:suffix("_total", "my_metric_total").
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to