Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add cli option & system property to enable json only logging for http json service #9725

Merged
merged 1 commit into from
May 20, 2021
Merged

Add cli option & system property to enable json only logging for http json service #9725

merged 1 commit into from
May 20, 2021

Conversation

realvictorprm
Copy link
Contributor

@realvictorprm realvictorprm commented May 18, 2021 •
edited
Loading

changelog_begin

  • [Ledger HTTP Json Service] Logging output can now be in JSON either via providing the cli option --log-encoder json or via setting the env var LOG_FORMAT_JSON=true

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.

@realvictorprm realvictorprm changed the title Add cli option & system property to enable json only logging Add cli option & system property to enable json only logging for http json service May 18, 2021
@realvictorprm realvictorprm marked this pull request as draft May 18, 2021 10:58
stefanobaghino-da
stefanobaghino-da suggested changes May 18, 2021
View reviewed changes
Copy link
Contributor

@stefanobaghino-da stefanobaghino-da left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically good, but I think it's quite important to allow users to have the context printed out if they choose a non-structured logger.

ledger-service/cli-opts/src/main/scala/cliopts/Logging.scala Outdated Show resolved Hide resolved
ledger-service/http-json/src/main/scala/com/digitalasset/http/Main.scala Outdated Show resolved Hide resolved
libs-scala/contextualized-logging/src/main/scala/com/digitalasset/logging/LeveledLogger.scala Show resolved Hide resolved
@realvictorprm realvictorprm force-pushed the jsonLoggingHttpJsonApi branch 4 times, most recently from 9f90d39 to 542a1a6 Compare May 18, 2021 13:30
@realvictorprm realvictorprm force-pushed the jsonLoggingHttpJsonApi branch 2 times, most recently from bee1120 to af4956c Compare May 18, 2021 15:19
@realvictorprm realvictorprm marked this pull request as ready for review May 18, 2021 16:03
@realvictorprm realvictorprm force-pushed the jsonLoggingHttpJsonApi branch from 0e31f2a to 661bf8d Compare May 18, 2021 16:25
@realvictorprm realvictorprm force-pushed the jsonLoggingHttpJsonApi branch from 661bf8d to 220c588 Compare May 19, 2021 09:36
@realvictorprm realvictorprm requested a review from a user May 19, 2021 09:36
@realvictorprm realvictorprm force-pushed the jsonLoggingHttpJsonApi branch from 220c588 to 351d8a6 Compare May 19, 2021 10:56
@realvictorprm realvictorprm force-pushed the jsonLoggingHttpJsonApi branch from 351d8a6 to e25f331 Compare May 19, 2021 11:33
@realvictorprm realvictorprm force-pushed the jsonLoggingHttpJsonApi branch from e25f331 to 435ed13 Compare May 19, 2021 11:38
cocreature
cocreature approved these changes May 19, 2021
View reviewed changes
Copy link
Contributor

@cocreature cocreature left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thank you!

@realvictorprm realvictorprm force-pushed the jsonLoggingHttpJsonApi branch from 435ed13 to d38a62e Compare May 19, 2021 12:42
@realvictorprm
Copy link
Contributor Author

OK the CI hates me I think but let's see after another run 🤔

@realvictorprm realvictorprm force-pushed the jsonLoggingHttpJsonApi branch from d38a62e to 87e32c3 Compare May 19, 2021 15:42
@realvictorprm
Add cli option & system property to enable json only logging for http…
d59096f 
… json service

changelog_begin
- [Ledger HTTP Json Service] Logging output can now be in JSON either via providing the cli option `--log-encoder json` or via setting the env var `LOG_FORMAT_JSON=true`
changelog_end
@realvictorprm realvictorprm force-pushed the jsonLoggingHttpJsonApi branch from 87e32c3 to d59096f Compare May 20, 2021 09:01
@stefanobaghino-da stefanobaghino-da dismissed their stale review May 20, 2021 13:27

Points addressed

@mergify mergify bot merged commit beb2d13 into main May 20, 2021
@mergify mergify bot deleted the jsonLoggingHttpJsonApi branch May 20, 2021 13:28
azure-pipelines bot pushed a commit that referenced this pull request May 26, 2021
release 1.14.0-snapshot.20210525.7017.0.2710fad0
aca9db1 
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.

@aherrmann-da is in charge of this release.

Commit log:
```
2710fad Normalize rollback nodes when a transaction is constructed (#9712)
646c956 new windows signing (#9786)
cae4292 Upgrade rules-nodes to version 3.5.0 (#9635)
a336363 ghc-lib: update (#9742)
8ef9361 Scenario: Test ArithemticError (#9757)
ad529e9 http-json-perf: querying larger ACSes with different data patterns (#9530)
0b24655 Fix archive calls in ExceptionsIT (#9787)
abb7142 Update LF spec for exceptions. (#9784)
d208979 Make succ/pred throw an ArithmeticError on overflow. (#9783)
cd2ed07 Rename ContractError to PreconditionFailed (#9782)
1b428be Add ledger API test tool tests for rollback projections (#9778)
fb6f72c debug signtool on main (#9780)
459de1c update NOTICES file (#9781)
61293c3 maybe fix azuresigntool invocation (#9779)
66b8f19 DPP-406 split append only migration (#9777)
45bca6e test_windows_signing: install for u (#9776)
ed36caa Make use of nameOf for naming telemetry traces in participant-integration-api (#9762)
6f20d78 Makes sandbox-classic compression configurable (#9770)
b8afa02 test_windows_signing: add quotes (#9773)
4af6608 fix signing machine (#9772)
5717356 fix test_windows_signing job name (#9771)
5855a99 LF: Drop CONTRACT_ERROR (#9754)
f5c5b63 prepare for EV Windows signing (#9758)
88c83ec Mark //daml-assistant/daml-helper:test-deployment as flaky (#9769)
fa0815b Example export (#9756)
3b33397 update NOTICES file (#9767)
4f97a4d Avoid test log collisions across platforms (#9763)
6f631da Use munit for diffs in ledger-api-test-tool (#9765)
92568eb Make hikari connection pool timeout configurable (#9750)
38cb8b4 Fix flaky CMConcurrentSetConflicting conformance test (#9766)
d2bce16 DPP-335 Implement data migration for the append-only schema (#9481)
765d7e3 Throw ContractError in template precondition field (#9760)
f5a03b3 Stop leaking Sdk installations from assistant integration tests (#9751)
a277560 LF: change format of ArithmeticError message (#9761)
90180eb Make sure we don’t start leaking directories in compat tests (#9759)
beb2d13 Add cli option & system property to enable json only logging for http json service (#9725)
3827f82 Fix consumedBy in rollback nodes for scenario service (#9746)
2ba5ca7 Make DA.Assert throw AssertionFailed instead of GeneralError (#9747)
b9c36c9 Disable per commit windows compat tests (#9755)
b1ca310 ledger: Damlification of Scala files (#9667)
40cf26f Log improvements for parallel indexer (#9752)
3bf6531 ledger-service, navigator: Damlification of Scala files (#9669)
f947c02 Extend ConfigManagementServiceIT with tests on generation checking (#9753)
f5c84a2 Daml-LF: Damlification of Scala/Haskell files (#9666)
0d931b2 Speedy: implement Arithmetic builtin exceptions (#9653)
5f954da Fix flakiness in hot reload tests (#9748)
40b2381 Compiler: clean convertion builtin name (#9724)
7b1a776 Dpp 384 todo cleanup postgres storage backend (#9727)
2e9bb21 Engine: clean conversion builtin names (#9726)
8efdde6 FutureResourceOwner --> ResourceOwner.forTry (#9720)
1794a6f Refactor GenerateStablePackages (#9744)
14cfe99 update compat versions for 1.14.0-snapshot.20210518.6953.0.a6c7b86a (#9741)
83d60f0 more frequent Windows cache cleanup (#9743)
10177d2 Add golden test for example Daml ledger export (#9732)
bb5dd4c Fix locking of envScenarioContexts (#9736)
fcbba1c LF: rename convertion builtins to be more obvious (#9716)
b55613b Parallelize the data-dependencies test. (#9733)
3209188 Add a ledger model page for exceptions. (#9396)
6245e34 Drop damlc integration test todo (#9735)
ae523c7 `ledger-api-bench-tool` - support for multiple streams [DPP-372] (#9691)
0f6f654 rotate release duty after 1.14.0-snapshot.20210518.6953.0.a6c7b86a (#9738)
123e142 release 1.14.0-snapshot.20210518.6953.0.a6c7b86a (#9737)
2dbfe43 Fix conversion of TryContextInfo in scenario service (#9731)
e375fef update NOTICES file (#9739)
```
Changelog:
```
[jdbc ledger] increase default hikari connection pool timeout to 2s from 250ms
- [Ledger HTTP Json Service] Logging output can now be in JSON either via providing the cli option `--log-encoder json` or via setting the env var `LOG_FORMAT_JSON=true`
- [Daml Standard Library] `assert`, `(===)`, and other assertion functions (see DA.Assert) now use a new `CanAssert` typeclass constraint instead of `CanAbort`, in preparation for exceptions support.
- [docs] The Daml ledger model has been updated to describe the changes introduced by the new exceptions feature. See  here: https://docs.daml.com/concepts/ledger-model/ledger-exceptions.html
- [Integration Kit] - Created the ledger-api-bench-tool prototype for benchmarking ledger transaction streaming capabilities
- [Integration Kit] - added metrics to the ledger-api-bench-tool
- [Integration Kit] - support for multiple streams in the ledger-api-bench-tool
```

CHANGELOG_BEGIN
CHANGELOG_END
realvictorprm added a commit that referenced this pull request Nov 16, 2021
@realvictorprm
Add cli option & system property to enable json only logging for http…
697220d 
… json service (#9725)

changelog_begin
- [Ledger HTTP Json Service] Logging output can now be in JSON either via providing the cli option `--log-encoder json` or via setting the env var `LOG_FORMAT_JSON=true`
changelog_end
realvictorprm added a commit that referenced this pull request Nov 16, 2021
@realvictorprm
Add cli option & system property to enable json only logging for http…
9b43cfc 
… json service (#9725)

changelog_begin
- [Ledger HTTP Json Service] Logging output can now be in JSON either via providing the cli option `--log-encoder json` or via setting the env var `LOG_FORMAT_JSON=true`
changelog_end
realvictorprm added a commit that referenced this pull request Nov 16, 2021
@realvictorprm
Add cli option & system property to enable json only logging for http…
9cc3512 
… json service (#9725)

changelog_begin
- [Ledger HTTP Json Service] Logging output can now be in JSON either via providing the cli option `--log-encoder json` or via setting the env var `LOG_FORMAT_JSON=true`
changelog_end
realvictorprm added a commit that referenced this pull request Nov 17, 2021
@realvictorprm
Add cli option & system property to enable json only logging for http…
0e00228 
… json service (#9725)

changelog_begin
- [Ledger HTTP Json Service] Logging output can now be in JSON either via providing the cli option `--log-encoder json` or via setting the env var `LOG_FORMAT_JSON=true`
changelog_end
realvictorprm added a commit that referenced this pull request Nov 18, 2021
@realvictorprm
Add cli option & system property to enable json only logging for http…
af92238 
… json service (#9725)

changelog_begin
- [Ledger HTTP Json Service] Logging output can now be in JSON either via providing the cli option `--log-encoder json` or via setting the env var `LOG_FORMAT_JSON=true`
changelog_end
realvictorprm added a commit that referenced this pull request Nov 18, 2021
@realvictorprm
[Backport] Add cli option & system property to enable json only loggi…
b7fc305 
…ng for the json api (#11717)

* Add cli option & system property to enable json only logging for http json service (#9725)

changelog_begin
- [Ledger HTTP Json Service] Logging output can now be in JSON either via providing the cli option `--log-encoder json` or via setting the env var `LOG_FORMAT_JSON=true`
changelog_end

* Load the correct logback file for the http json service respecting the deployment situation (#9938)

* Load the correct logback file for the http json service respecting the deployment situation

changelog_begin

[HTTP-JSON]
- fixed that json log output could not be enabled via cli options except via usage of env vars

changelog_end

* Move import statement, remove some braces and reformat

* Move system prop dependend logic to cliopts Logging.scala

* Remove PathKind type in cliopts Logging.scala as it's not necessary anymore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cocreature cocreature cocreature approved these changes

@aherrmann-da aherrmann-da Awaiting requested review from aherrmann-da

@S11001001 S11001001 Awaiting requested review from S11001001

@gerolf-da gerolf-da Awaiting requested review from gerolf-da

@mziolekda mziolekda Awaiting requested review from mziolekda

@nmarton-da nmarton-da Awaiting requested review from nmarton-da

@rautenrieth-da rautenrieth-da Awaiting requested review from rautenrieth-da

@tudor-da tudor-da Awaiting requested review from tudor-da

@fabiotudone-da fabiotudone-da Awaiting requested review from fabiotudone-da

@miklos-da miklos-da Awaiting requested review from miklos-da

@stefanobaghino-da stefanobaghino-da Awaiting requested review from stefanobaghino-da

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@realvictorprm @cocreature @stefanobaghino-da