[JSON-API] Add moar timing metrics #10045

realvictorprm · 2021-06-17T11:59:27Z

Solves part of #10020

I want to add the db metrics with a seperate PR as this seems to be a bit more complex (also because I have not worked in that part of the code yet).

changelog_begin

[JSON-API] Timing metrics are now available for party management, package management, command submission and query endpoints.
[JSON-API] Also added a timing metric for parsing and decoding of incoming json payloads

changelog_end

Pull Request Checklist

Read and understand the contribution guidelines
Include appropriate tests
Set a descriptive title and thorough description
Add a reference to the issue this PR will solve, if appropriate
Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
Normal production system change, include purpose of change in description

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

changelog_begin - [JSON-API] Timing metrics are now available for party management, package management, command submission and query endpoints. - [JSON-API] Also added a timing metric for parsing and decoding of incoming json payloads changelog_end

stefanobaghino-da · 2021-06-17T12:15:08Z

ledger/metrics/src/main/scala/com/daml/metrics/Metrics.scala

+      // Meters how long processing of a command submission request takes
+      val commandSubmissionTimer: Timer = registry.timer(Prefix :+ "command_submission_timing")
+      // Meters how long processing of a query request takes
+      val queryTimer: Timer = registry.timer(Prefix :+ "query_timing")
+      // Meters how long processing of a party management request takes
+      val partyManagementTimer: Timer = registry.timer(Prefix :+ "party_management_timing")
+      // Meters how long processing of a package management request takes
+      val packageManagementTimer: Timer = registry.timer(Prefix :+ "package_management_timing")
+      // Meters how long parsing and decoding of incoming json payload takes
+      val incomingJsonParsingAndValidationTimer: Timer =
+        registry.timer(Prefix :+ "incoming_json_parsing_and_validation_timing")


It's probably fine to have all command endpoints tracked by a single timer, but shouldn't we rather have more fine-grained metrics to allow users to differentiate between the time taken to, say, make a query or fetch request?

I can do that, I'm just unsure about what is most useful. Maybe I should just go the route "More metrics = better" 😄

Not necessarily, I think that grouping commands together in sensible, but I would at least differentiate for things that clearly achieve different things, like between uploading and downloading a package.

did split them up with 33842a5

ledger-service/http-json/src/main/scala/com/digitalasset/http/Endpoints.scala

realvictorprm · 2021-06-17T12:42:31Z

Darn, CI failes due to formatting. How could I forgot to run a format before commiting 😠

stefanobaghino-da · 2021-06-17T12:56:15Z

Darn, CI failes due to formatting. How could I forgot to run a format before commiting

I have

#!/bin/sh

./ci/check-changelog.sh && ./fmt.sh --diff --test

as my .git/hooks/pre-push, this way I can't push changes that wouldn't pass platform-agnostic checks.

tudor-da · 2021-06-17T12:59:53Z

Darn, CI failes due to formatting. How could I forgot to run a format before commiting

I have
#!/bin/sh

./ci/check-changelog.sh && ./fmt.sh --diff --test
as my .git/hooks/pre-push, this way I can't push changes that wouldn't pass platform-agnostic checks.

ha, nice one 👍

realvictorprm · 2021-06-17T13:17:19Z

going to try that out too, thanks @stefanobaghino-da 😄

realvictorprm · 2021-06-17T14:29:08Z

ledger-service/http-json/src/main/scala/com/digitalasset/http/Endpoints.scala

      case req @ HttpRequest(GET, Uri.Path("/v1/query"), _, _, _) =>
        (implicit lc => httpResponse(retrieveAll(req)))
      case req @ HttpRequest(POST, Uri.Path("/v1/query"), _, _, _) =>
        (implicit lc => httpResponse(query(req)))
+    }
+    val fetchDispatch: DispatchFun = {


I think I'm going to merge the single case matches here into one because it's also weird to have an extra declaration for them then.

on the other hand, mixing stuff together when it's split up that cleanly isn't looking that much better either.

realvictorprm · 2021-06-17T14:29:41Z

ledger-service/http-json/src/main/scala/com/digitalasset/http/Endpoints.scala

      case req @ HttpRequest(POST, Uri.Path("/v1/parties/allocate"), _, _, _) =>
-        (implicit lc => httpResponse(allocateParty(req)))
+        (


All of this weird formatting just because scalafmt needs these braces, otherwise it fails with a parser error -.-

I'll take weird formatting over formatting discussions any time. 🙂

cocreature

Thanks!

ledger-service/http-json/src/main/scala/com/digitalasset/http/Endpoints.scala

realvictorprm · 2021-06-21T10:06:37Z

I think mac is being flaky again

So much wow....

realvictorprm · 2021-06-21T10:16:51Z

/AzurePipelines

azure-pipelines · 2021-06-21T10:16:55Z

Supported commands help: Get descriptions, examples and documentation about supported commands Example: help "command_name" list: List all pipelines for this repository using a comment. Example: "list" run: Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run. Example: "run" or "run pipeline_name, pipeline_name, pipeline_name" where: Report back the Azure DevOps orgs that are related to this repository and org Example: "where" See additional documentation.

realvictorprm · 2021-06-21T10:17:32Z

/AzurePipeline run macOS

azure-pipelines · 2021-06-21T10:17:37Z

No pipelines are associated with this pull request.

realvictorprm · 2021-06-21T10:18:01Z

It's taking way longer than I want, excuse my impatience

realvictorprm · 2021-06-21T10:18:45Z

/AzurePipeline list

azure-pipelines · 2021-06-21T10:18:49Z

CI/CD Pipelines for this repository: digital-asset.daml-cron digital-asset.daml-daily-compat digital-asset.daml-ci-cron-wednesday digital-asset.daml-ci-cron-tuesday PRs

realvictorprm · 2021-06-21T10:19:15Z

/AzurePipeline run PRs

azure-pipelines · 2021-06-21T10:19:30Z

Azure Pipelines successfully started running 1 pipeline(s).

cocreature · 2021-06-21T10:20:27Z

/azp run reruns the whole pipeline. Usually you are better off waiting for the build to finish and rerunning only the jobs that failed via the GH ui.

realvictorprm · 2021-06-21T10:22:48Z

I built on build caching for the successful run pipelines, however maybe it would have finished if I waited a bit longer. It just was kind of stuck in the notify_user stage.

realvictorprm · 2021-06-21T13:17:42Z

as the machines were broken I unfortunately need to restart that whole stuff.

realvictorprm · 2021-06-21T13:17:46Z

/AzurePipeline run PRs

azure-pipelines · 2021-06-21T13:18:01Z

Azure Pipelines successfully started running 1 pipeline(s).

@S11001001

This PR has been created by a script, which is not very smart and does not have all the context. Please do double-check that the version prefix is correct before merging. @S11001001 is in charge of this release. Commit log: ``` d867d90 define query offset semantics solely in terms of #9847 per-query offsets (#10071) 1f5aa44 [JSON-API] Add metrics for ledger command submission timing (#10076) 7eb2ce7 update compat versions for 1.15.0-snapshot.20210615.7169.0.adeba206 (#10028) c44f337 Simplify speedy benchmark (#10079) 58b1c4e Speedy: Refactor contract Id/Key callback (#10033) 8558c73 Upgrade canton to 0.25.0 based on daml 1.14.0 (#10074) e4c1c58 Run scenarios in off-ledger machine (#10070) 31a76a4 allow CI pools to use any zone (#10069) 0391f35 [JSON-API] Add db metrics & response construction metrics (#10068) ea089ad clean up local VSCode after release (#10065) 39a5890 fix 9_functional redirect (#10067) fb757d8 Handle visibility outside of speedy (#10056) b796695 Add per-query offsets to HTTP-JSON API (#9847) ab8bac5 [In-memory fan-out] BuffersUpdater subscribes from the ledger end (#10037) e458529 [JSON-API] Add moar timing metrics (#10045) e12a449 s/DABL/Daml Hub/ (#10062) a205d0e Enriched conformance tests for transactions (trees) streams (#9977) b98ad91 Bump test sha (#10055) f3106c3 I9996 value limits it fails oracle (#10027) febca5d Use ScenarioRunner.submit in Daml Script (#10053) 3113702 index json-api signatories and observers on Oracle (#9895) cb3a42a update NOTICES file (#10047) 2f51869 Allow constraints in any position in data-deps. (#10049) 7dfa36f pass supportedJdbcDriverNames implicitly, and only to what actually needs it (#10036) 0fbc1ee H2 support for append only schema (part2) [DPP-394] (#10030) cd2fda6 Fix broken package.json config (#10044) 10ab05f Fix vsce publishing (#10043) 090dd95 rotate release duty after 1.14.0-snapshot.20210615.7169.0.adeba206 (#10019) eee484b Implemented in-memory buffers prunning in BaseLedger (#9936) b96639e Do not terminate streams when no end-offset given in `ledger-api-bench-tool` [DPP-422] (#10035) 2b915e9 Split scenario & ledger execution (#10039) 591176c Remove SqlSequence, cleanup code (#10042) 8ea240f Release SDK 1.14.0 (#10041) 3532460 LF: Structure Preprocessing Errors (#10013) 1852830 [JSON-API] Concurrent query etc. metrics (#10031) 321c4da test different template-ID frequencies in the "mega" perf scenario (#10016) 993591e truncate party name for 64 chars for multi-valued submitters to avoid… (#10000) 7a2a349 Log context of all updates written to the database (#10010) 4005c84 release 1.15.0-snapshot.20210615.7169.0.adeba206 (#10022) d1aa256 Force css-what resolution in compiler/daml-extension & fully regenera… (#10025) 0fbfd99 DPP-394 h2 support for append only schema (#10009) 32d70cf Revert "release 1.14.0-snapshot.20210615.7169.0.adeba206 (#10018)" (#10023) cfee5a2 [JSON-API] Add information from the jwtpayload to the logging context (#9995) fe63825 release 1.14.0-snapshot.20210615.7169.0.adeba206 (#10018) ``` Changelog: ``` - [JSON-API] Timing metrics which measure how long the processing of a command submission request takes on the ledger are now available - [JSON-API] The database operations (regardless of in-memory or postgres) contain now metrics for fetching contracts by id or key (seperate metrics foreach) - [JSON-API] The time for a repsonse payload construction of a request is now also tracked in a metric [HTTP/JSON API] The streaming query endpoint now accepts to override the offset for each query using the `offset` field. You can read more about it on the documentation. - [JSON-API] Timing metrics are now available for party management, package management, command submission and query endpoints. - [JSON-API] Also added a timing metric for parsing and decoding of incoming json payloads - [JSON API] The Oracle database schema has changed; if using ``--query-store-jdbc-config``, you must rebuild the database by adding ``,createSchema=true``. See #9895. - [JSON-API] The metrics which describe the amount of these concurrent events is now available: - running http request - command submissions - package allocations - party allocations [metrics] Limit size of multi party metrics to ease consumption For every update in the index db log the full context at the INFO level. - [JSON API] For applicable requests actAs, readAs, applicationId & ledgerId are included in the log context ``` CHANGELOG_BEGIN CHANGELOG_END

realvictorprm added 2 commits June 17, 2021 13:50

[JSON-API] Add more timing metrics

8e58411

changelog_begin - [JSON-API] Timing metrics are now available for party management, package management, command submission and query endpoints. - [JSON-API] Also added a timing metric for parsing and decoding of incoming json payloads changelog_end

Add comments to new metrics

77801f7

realvictorprm requested review from cocreature, gerolf-da, mziolekda, nmarton-da, rautenrieth-da, S11001001, stefanobaghino-da and tudor-da as code owners June 17, 2021 11:59

realvictorprm changed the title ~~Http json/moar timing metrics~~ [JSON-API] Add moar timing metrics Jun 17, 2021

realvictorprm added the component/json-api HTTP JSON API label Jun 17, 2021

stefanobaghino-da reviewed Jun 17, 2021

View reviewed changes

Split metrics up more & remove obsolete metric

33842a5

realvictorprm commented Jun 17, 2021

View reviewed changes

cocreature approved these changes Jun 21, 2021

View reviewed changes

ledger-service/http-json/src/main/scala/com/digitalasset/http/Endpoints.scala Outdated Show resolved Hide resolved

Split up timers for query endpoints

f496c2c

realvictorprm added the automerge label Jun 21, 2021

mergify bot merged commit e458529 into main Jun 21, 2021

mergify bot deleted the http-json/moar-timing-metrics branch June 21, 2021 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JSON-API] Add moar timing metrics #10045

[JSON-API] Add moar timing metrics #10045

realvictorprm commented Jun 17, 2021

stefanobaghino-da Jun 17, 2021

realvictorprm Jun 17, 2021

stefanobaghino-da Jun 17, 2021

realvictorprm Jun 17, 2021

realvictorprm commented Jun 17, 2021

stefanobaghino-da commented Jun 17, 2021

tudor-da commented Jun 17, 2021

realvictorprm commented Jun 17, 2021

realvictorprm Jun 17, 2021

realvictorprm Jun 21, 2021

realvictorprm Jun 17, 2021

stefanobaghino-da Jun 17, 2021

cocreature left a comment

realvictorprm commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

azure-pipelines bot commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

azure-pipelines bot commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

azure-pipelines bot commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

azure-pipelines bot commented Jun 21, 2021

cocreature commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

azure-pipelines bot commented Jun 21, 2021

[JSON-API] Add moar timing metrics #10045

[JSON-API] Add moar timing metrics #10045

Conversation

realvictorprm commented Jun 17, 2021

Pull Request Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

realvictorprm commented Jun 17, 2021

stefanobaghino-da commented Jun 17, 2021

tudor-da commented Jun 17, 2021

realvictorprm commented Jun 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cocreature left a comment

Choose a reason for hiding this comment

realvictorprm commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

azure-pipelines bot commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

azure-pipelines bot commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

azure-pipelines bot commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

azure-pipelines bot commented Jun 21, 2021

cocreature commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

realvictorprm commented Jun 21, 2021

azure-pipelines bot commented Jun 21, 2021