Log duplicate string map option keys

Summary:
Review context

This child responds to the following comment from Anton (`alikhtarov`) on D113813215:

> Consider logging on duplicates/empty keys to MC_FAILURES or similar

What this addresses:

`parseStringMapOption()` resolves duplicate keys silently — regular `string_map` options keep the first value, `--additional-config-params` keeps the last. Neither policy surfaced the discarded value, so a config typo, or a template that substitutes into a key already present (e.g. `%PROC_NAME%:a,cmd:b` when the process is named `cmd`), just disappeared.

Duplicates are now logged as a `Category::kInvalidOption` failure naming the key, the value used, and the value ignored. Detection uses a `find()` before insert instead of branching between `emplace()` and `insert_or_assign()`, so both parsing policies share one log statement. Which value wins is unchanged.

Where these land, and a caveat:

`LOG_FAILURE` reaches `mc_failures` through `ScubaFailureLogger` once `initFailureLogger()` has registered the `scubaMcFailure` handler. String map options are parsed before that point — `updateFromDict()` and the `--additional-config-params` merge both run earlier in `setupStandaloneMcrouter()`, and embedded clients build `McrouterOptions` before `CarbonRouterInstance::init()`. So at standalone startup these reach stderr/glog, not scuba.

That is the same behavior as every other option-parse failure: the `Unknown option name` `LOG_FAILURE` in `updateFromDict()` has the identical ordering. I prototyped deferring just these reports through a collect-and-flush helper, but it meant process-wide state and a lock that no other log site here needs, for one call site. The fix that actually pays for itself is buffering pre-handler failures inside `LogFailure` and replaying them to handlers as they register, which fixes the whole class at once — happy to write that as a follow-up if you want duplicates in scuba specifically.

What this does not address:

Empty pairs, the other half of the comment. They already throw for regular `string_map` options; the silent skip is limited to `--additional-config-params`. Happy to add that too.

Reviewed By: djvaporize

Differential Revision: D114799408

fbshipit-source-id: 76daf3d0029f7aed85e56f2901db91fb504fc556

Author: Vrishal Kulkarni

mcrouter/options.cpp

@@ -51,10 +51,21 @@ unordered_map<string, string> parseStringMapOption(
     auto key = substituteTemplates(pair.subpiece(0, delimiter).str());
     checkLogic(!key.empty(), "Empty key in string map pair: '{}'.", pair);
     auto value = substituteTemplates(pair.subpiece(delimiter + 1).str());
-    if (parseOptions.overwriteDuplicateKeys) {
-      result.insert_or_assign(std::move(key), std::move(value));
-    } else {
+    auto duplicate = result.find(key);
+    if (duplicate == result.end()) {
       result.emplace(std::move(key), std::move(value));
+      continue;
+    }
+    const bool overwrite = parseOptions.overwriteDuplicateKeys;
+    LOG_FAILURE(
+        "mcrouter",
+        failure::Category::kInvalidOption,
+        "Duplicate key '{}' in string map option: using value '{}', ignoring '{}'.",
+        key,
+        overwrite ? value : duplicate->second,
+        overwrite ? duplicate->second : value);
+    if (overwrite) {
+      duplicate->second = std::move(value);
     }
   }
   return result;