Fix type validation in configstore DC client value updating

[charlese-instaclustr]

Fixed a misleading check that was allowing config_store_client#UpdateValue to return no error, despite actually failing the operation and corrupting the dynamic config storage in the process.

Added a more verbose log message when the type check fails, to aid users in understanding the issue.

Why?

This is a temporary change mitigating a more widespread issue described in #5109 -- in summary, inconsistent assumptions about input data types across different implementations of the functions outlined in the Dynamicconfig.Client interface lead to instances in the codebase where the success / failure of calls to a dynamic config client depends on its specific implementation.

By making this change, the config_store_client will no longer attempt to update its values when itsUpdateValue implementation is passed data in the format currently expected by the file_based_config client. This is an improvement because it will stop users inadvertently corrupting their dynamic config, and by consulting the updated log message, they will be informed that they can update their config via the CL instead.

However, long-term, a priority should be addressing issue #5109 and eliminating the inconsistency in this DC client interface.

How did you test it?

Tested locally.

• Ran Advanced Visibility Cadence server with the config store dynamic config client enabled (config available in development.yaml)
• Added an initial value to the dynamic config for frontend.validSearchAttributes in the CLI: cadence admin config updc --dynamic_config_name frontend.validSearchAttributes --dynamic_config_value '{"Value": '{"testAttribute": 2}',"Filters":[]}'
• Verified that the config value has been added: cadence admin config listdc
• Triggered the AddSearchAttribute handler: cadence admin cluster asa --search_attr_key testAttribute2 --search_attr_type 2. Note that this handler calls the UpdateValue function with the problematic input data format.
• Reran: cadence admin config listdc. The prior set value for frontend.validSearchAttributes remains the same; by constrast, in the current master implementation, the prior set value would be gone.
• Checked the Cadence logs, and confirmed that the updated log message shows.

Potential risks

No notable risks from this change; we are simply fixing a type validation so that it serves its intended purpose.

Release notes

Not notable.

Documentation Changes

N/A

[allenchen2244]

@charlese-instaclustr maybe test is failing somehow?

[charlese-instaclustr]

@allenchen2244 good pickup, apologies, the tests picked up an edge case where the update value is nil. Added handling for the edge case to my change if you could rereview please. Thanks

[charlese-instaclustr]

Hi @allenchen2244 , would you be able to give me a hand by rerunning the build and tests please? Thanks

[coveralls]

Pull Request Test Coverage Report for Build 0186f18b-64da-446c-b19d-eadac9691f7c

• 2 of 2 (100.0%) changed or added relevant lines in 2 files are covered.
• 9 unchanged lines in 4 files lost coverage.
• Overall coverage increased (+0.03%) to 57.109%

---

Files with Coverage Reduction	New Missed Lines	%
common/task/fifoTaskScheduler.go	2	84.54%
common/task/weightedRoundRobinTaskScheduler.go	2	88.6%
common/util.go	2	52.31%
service/history/execution/mutable_state_task_refresher.go	3	56.65%

Totals
Change from base Build 0186f0f3-f0db-4e0e-ab38-a4ae3852f931:	0.03%
Covered Lines:	85273
Relevant Lines:	149317

---

💛 - Coveralls