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

0034 message tracing #464

Merged
merged 9 commits into from
Oct 9, 2024
Merged

Conversation

ianco
Copy link
Member

@ianco ianco commented Apr 8, 2020

Discussion points from the Aires call today (April 8), for posterity:

Trace decorator:

  • whether the agent chooses to log trace events to the given "target" should be optional, i.e. the Agent can choose whether to log to the requested target, or to the pre-configured target (asking for trace events in this manner represents a potential data leak)
  • I believe "full_trace" should always be "true" and is therefore redundant (if tracing is requested on a message or exchange you will always want the full scope of trace events)

Trace Report:

  • added "thread_id", I believe this should always be provided if possible (or can be left blank if not available)
  • added two versions of timestamp (epoch seconds and human-readable date/time) (note that when we use fluentd as a "target" it adds its own timestamp of when the event was received)
  • added may the sequential suffix on "msg_id" (“... in the form X.1, X.2, X.3”) - didn't think this was necessary, as the "thread_id" provides correlation and fluentd provides a central timestamp) - so made it an optional feature Agents can choose to implement (with aca-py I don't think the additional effort is warranted)

ianco added 4 commits March 18, 2020 13:18
Signed-off-by: Ian Costanzo <ian@anon-solutions.ca>
Signed-off-by: Ian Costanzo <ian@anon-solutions.ca>
Signed-off-by: Ian Costanzo <ian@anon-solutions.ca>
@dhh1128
Copy link
Contributor

dhh1128 commented May 22, 2020

I feel like this PR combines some excellent improvements with some undesirable shifts in the intent of tracing. I would like to tease them apart in an interactive discussion. Can we talk further before the PR moves forward as a unit?

@ianco
Copy link
Member Author

ianco commented May 29, 2020

@dhh1128 for sure. Do you know who else has implemented tracing (or is planning to)? They should be involved in any discussions of this rfc.

ianco and others added 4 commits July 27, 2020 07:25
@TelegramSam TelegramSam marked this pull request as ready for review October 9, 2024 14:45
@TelegramSam
Copy link
Contributor

Discussed 20241009. Has been built into ACA-Py.

@TelegramSam TelegramSam merged commit e70c296 into hyperledger:main Oct 9, 2024
2 checks passed
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