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.

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

CDNC-2088 #5094

Merged
merged 4 commits into from
Feb 14, 2023
Merged

CDNC-2088 #5094

merged 4 commits into from
Feb 14, 2023

Conversation

bowenxia
Copy link
Contributor

What changed?
Add AttemptCount into log

Why?
Currently we have metrics for number-of-attempts and have alerts on them, but finding the associated workflows can be tricky due to other logs also existing / temporary issues that self-recover happening all the time.

How did you test it?
Unit test

Potential risks
no

Release notes

Documentation Changes

@CLAassistant
Copy link

CLAassistant commented Feb 13, 2023

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

coveralls commented Feb 13, 2023

Pull Request Test Coverage Report for Build 01865217-44ea-4053-acb4-6c4de3638ef3

  • 0 of 8 (0.0%) changed or added relevant lines in 3 files are covered.
  • 116 unchanged lines in 14 files lost coverage.
  • Overall coverage decreased (-0.03%) to 57.18%

Changes Missing Coverage Covered Lines Changed/Added Lines %
service/history/task/task.go 0 1 0.0%
common/log/tag/tags.go 0 2 0.0%
service/history/task/cross_cluster_task.go 0 5 0.0%
Files with Coverage Reduction New Missed Lines %
common/log/tag/tags.go 1 51.46%
service/history/queue/timer_queue_processor_base.go 1 77.26%
service/history/task/cross_cluster_task.go 1 67.72%
common/persistence/persistenceMetricClients.go 2 55.97%
service/matching/taskListManager.go 2 78.72%
common/persistence/statsComputer.go 3 94.64%
common/persistence/serialization/parser.go 4 65.41%
common/persistence/serialization/thrift_decoder.go 4 61.22%
service/history/task/fetcher.go 4 91.24%
common/persistence/sql/workflowStateMaps.go 12 85.34%
Totals Coverage Status
Change from base Build 01864c84-f59b-4822-a8e4-1c6ff770d9df: -0.03%
Covered Lines: 84971
Relevant Lines: 148602

💛 - Coveralls

@neil-xie
Copy link
Contributor

from your note, do you need to add the tag here https://github.com/uber/cadence/blob/master/service/history/task/cross_cluster_task.go#L362

@bowenxia
Copy link
Contributor Author

@neil-xie Yes you are right! Nice catch. I totally missed that one. I've searched all other uses of the metric TaskAttemptTimerPerDomain, and made sure that everything was covered.

Copy link
Contributor

@neil-xie neil-xie left a comment

Choose a reason for hiding this comment

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

lg

Copy link
Contributor

@davidporter-id-au davidporter-id-au left a comment

Choose a reason for hiding this comment

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

nice!

@bowenxia bowenxia merged commit fbe72b3 into uber:master Feb 14, 2023
@bowenxia bowenxia deleted the test branch February 14, 2023 23:55
bowenxia added a commit to bowenxia/cadence that referenced this pull request Feb 23, 2023
bowenxia added a commit that referenced this pull request Feb 23, 2023
bowenxia added a commit that referenced this pull request Mar 6, 2023
…dy into Attribute for auditing (#5124)

* Revert "CDNC-2088 (#5094) Add attempt-count to task processing logs"

This reverts commit fbe72b3.

* experiment on adding audit info (has errorsgit status)

* move AuditInfo into Attr

* implement requestBody Struct, implement it using requests, add it into Attribute to use this field for auditing manual accesses

* change struct name to be ManualRequestBody to avoid conflict

* change any to interface to solve a compiling error

* rerun make go-generate && make fmt && make lint && make copyright for auto generated code

* change the PII-filtered request to still be a requset instead of a map

* add a utility function to check if access is manual, add request body into Attribute

* update with make go-generate && make fmt && make lint && make copyright

* update comments and naming

* change Sprintf to marshal

* add error handling for json.marshal, pass it to the caller of SerializeForLogging function

* add a new log tag for actor email address

* update interface function error handling

* pass in a pointer for serialization instead of a struct

* delete the isManual checks when creating Attributes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants