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

fix: metadata is properly built #42

Merged
merged 20 commits into from
Jan 6, 2025

Conversation

gentlementlegen
Copy link
Member

Resolves #41

@gentlementlegen
Copy link
Member Author

What are the changes

Currently the metadata headers are broken and do not display any info when used through the SDK. Also, Workers could not display any relevant information either. This is fixed by this pull-request (thank you @Keyrxng for the code head start).

I determined that the most important info we always use is:

  • instigator of the event (there are many times we wonder what happened because someone triggered the bot then deleted its comments so it looked strange)
  • the revision of the action / worker used
  • the URL to the run for actions because it is really hard to find the corresponding run, the URL to the dashboard for workers because it is very tedious to change the time range to match the comment within the issue

So now a header would look like the following:

  • UbiquityOS - @gentlementlegen - https://dash.cloudflare.com/1758eb0455a5d4e15ab52916ecb4991b/workers/services/view/ubiquity-os-command-query-user-development/production/observability/logs?granularity=0&time=%7B%22type%22%3A%22absolute%22%2C%22to%22%3A1734701838132%2C%22from%22%3A1734701718132%7D - queryUser - 6533250f-1e1a-4259-9f40-40f5c2018dbb for a Worker posted message
  • UbiquityOS - @gentlementlegen - https://github.com/Meniole/text-conversation-rewards/actions/runs/12431861924 - run - 1ad41aef8f27319064c2bf05fbad5ec3e3238e25 for an Action posted message

Which I think gives us the most important data and an easy way to find more detailed logs.

For convenience, I also added an option to post message raw that allow to post the body as sent in the function without our Logger formatting which is useful when you want to output complex data, like we do for text-conversation-reward results or even query-user when showing the user its wallet.

QA: Meniole/text-conversation-rewards#43 (not sure if you can see the metadata though but it's the one I copy pasted here).

Workers will need to upload one more secret that will help finding the user, and enable an option in their toml configuration that allows to fetch their ID, I will also carry out these changes if this is ok.

Also at this time, not every plugin uses our SDK to post so this has to be updated over time.

@gentlementlegen
Copy link
Member Author

Rfc @whilefoo @0x4007 I will fix Jest if the changes are ok.

README.md Outdated Show resolved Hide resolved
gentlementlegen and others added 2 commits December 29, 2024 14:53
Co-authored-by: アレクサンダー.eth <4975670+0x4007@users.noreply.github.com>
@gentlementlegen gentlementlegen marked this pull request as ready for review December 29, 2024 12:42
@0x4007
Copy link
Member

0x4007 commented Dec 30, 2024

Code changes look fine.

Interested to see the rendered metadata in a QA example.

@gentlementlegen
Copy link
Member Author

gentlementlegen commented Dec 30, 2024

@0x4007 Sorry didn't post QA because I figured you would not be able to see the metadata from my organization. Lemme display some results then.

Made it visible here: Meniole/text-conversation-rewards#44 (comment)

@0x4007
Copy link
Member

0x4007 commented Dec 30, 2024

Looks informative but what is the second field with the value of your username?

@gentlementlegen
Copy link
Member Author

@0x4007 It is the name of the person that instigated the run. I thought it was useful information because many times we were wondering why some comments were posted, and it was because the person who posted these comments deleted them afterwards.

@0x4007
Copy link
Member

0x4007 commented Dec 30, 2024

Sure we can leave it then.

src/server.ts Outdated Show resolved Hide resolved
src/server.ts Outdated Show resolved Hide resolved
src/comment.ts Show resolved Hide resolved
src/comment.ts Outdated Show resolved Hide resolved
src/comment.ts Show resolved Hide resolved
src/comment.ts Outdated Show resolved Hide resolved
@0x4007
Copy link
Member

0x4007 commented Jan 2, 2025

Actually the invoker field should be last for search reasons.

src/comment.ts Show resolved Hide resolved
@whilefoo
Copy link
Contributor

whilefoo commented Jan 5, 2025

@gentlementlegen I think you missed my comment

@gentlementlegen
Copy link
Member Author

@whilefoo Apologies I did miss it, addressing this now.

@gentlementlegen gentlementlegen merged commit f507eb1 into ubiquity-os:development Jan 6, 2025
3 checks passed
@gentlementlegen gentlementlegen deleted the fix/metadata branch January 6, 2025 00:07
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.

Properly set the metadata value of LogReturn
3 participants