Actualize Prometheus metrics collecting #632
Conversation
Actualize prometheus metrics
Updated CHANGELOG_DEV.md file
| @@ -50,6 +52,7 @@ func (handler AcraStructHandler) Decrypt(data []byte, context *base.DataProcesso | |||
| privateKeys, err := context.Keystore.GetServerDecryptionPrivateKeys(accessContext.GetClientID()) | |||
| defer utils.ZeroizePrivateKeys(privateKeys) | |||
| if err != nil { | |||
| base.AcrastructDecryptionCounter.WithLabelValues(base.LabelStatusFail).Inc() | |||
There was a problem hiding this comment.
hm, as I understand, it will increment twice for the acra-translator for its decrypt methods? If AT uses cryptohandler that uses this handler under the hood, then it will increment twice
There was a problem hiding this comment.
Yes, it will, nut these are two different metrics. One is a newly added one, and the second is to support backward compatibility.
tests/test.py
Outdated
| @@ -2176,6 +2176,7 @@ def testAcraServer(self): | |||
| 'acraserver_request_processing_seconds_bucket': {'min_value': 0}, | |||
|
|
|||
| 'acra_acrastruct_decryptions_total': {'min_value': 1}, | |||
| 'acra_decryptions_total': {'min_value': 1}, | |||
There was a problem hiding this comment.
will be great to add tests where all these metrics are used and we can validate them. Make a request to the table that is configured for all types of tokenization, for all types of encryption and decryption and validate, that all appropriate metrics were incremented. And same for acra-translator and its API. As I see, previously it just called HexFormatTest test case which forced simple acrastruct decryption. Now we have much more features) But to simplify tests, we can re-use some existing tests, imho.
There was a problem hiding this comment.
Added test additional cases for AcraSever. Left todo for AcraTranslator, as we are not supporting /v2 queries for AcraTranslator in integration tests.
Fixed after review
Added quard check for searchable tokenization observer
Added TODO for adding more metrics to track once we support v2 queries for translator in tests
| @@ -69,6 +70,10 @@ func (encryptor *TokenizeQuery) OnQuery(ctx context.Context, query base.OnQueryO | |||
| clientSession := base.ClientSessionFromContext(ctx) | |||
| bindSettings := queryEncryptor.PlaceholderSettingsFromClientSession(clientSession) | |||
| for _, item := range items { | |||
| if !item.Setting.IsTokenized() { | |||
There was a problem hiding this comment.
Should we add the same for searchable encryption?
Added quard case for searchable QueryObserver
Removed unnecessary teardown call
Actualizing Prometheus metrics collection.
Added new metrics:
success", "fail"], "type": [ "acrastruct", "acrablock", "acrablock_searchable", "acrastruct_searchable"],}success", "fail"], "type": [ "acrastruct", "acrablock", "acrablock_searchable", "acrastruct_searchable"],}success", "fail"], "token_type": "{token_type}"}success", "fail"], "token_type": "{token_type}"}Previously used metrics are left as they are to save backward compatibility and marked as deprecated.
Checklist
with new changes