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

cloud Function logging isn't working outside of the test scope anymore #3754

Closed
tsuf239 opened this issue Aug 9, 2023 · 5 comments
Closed
Labels
🐛 bug Something isn't working needs-discussion Further discussion is needed prior to impl 🎨 sdk SDK

Comments

@tsuf239
Copy link
Contributor

tsuf239 commented Aug 9, 2023

I tried this:

run examples/tests/sdk_tests/function/logging.w

This happened:

logged:

hello world
hello world

only logs in the test scope

I expected this:

should log:

hello world
log inside f1
log inside f2
log inside f1
hello world
log inside f1
log inside f2
log inside f1

logs in any scope

Is there a workaround?

No response

Component

SDK

Wing Version

No response

Node.js Version

No response

Platform(s)

No response

Anything else?

No response

Community Notes

  • Please vote by adding a 👍 reaction to the issue to help us prioritize.
  • If you are interested to work on this issue, please leave a comment.
@tsuf239 tsuf239 added 🐛 bug Something isn't working 🎨 sdk SDK labels Aug 9, 2023
@tsuf239 tsuf239 self-assigned this Aug 9, 2023
@monadabot monadabot added this to Wing Aug 9, 2023
@github-project-automation github-project-automation bot moved this to 🆕 New - not properly defined in Wing Aug 9, 2023
@tsuf239 tsuf239 moved this from 🆕 New - not properly defined to 🏗 In progress in Wing Aug 9, 2023
@Chriscbr
Copy link
Contributor

Chriscbr commented Aug 9, 2023

I can imagine how one might want to aggregate all of the logs from different functions that were "spawned" from a test... but I'm not sure it's clear what are the underlying rules for what logs are shown. For example:

bring cloud;
bring util;

let processor = new cloud.Function(inflight (m: str) => {
  log("processing: ${m}");
  counter.inc();
});

let queue = new cloud.Queue();
queue.setConsumer(inflight (m: str) => {
  processor.invoke(m);
});

let pusher = new cloud.Function(inflight (m: str) => {
  log("pushing: ${m}");
  queue.push(m);
});

let schedule = new cloud.Schedule(rate: 5s);
schedule.onTick(inflight () => {
  pusher.invoke("bloop");
});

test "message is processed" {
  pusher.invoke("floop");
  util.sleep(10s);
  assert(counter.peek() > 0);
  log("success!");
};

Here I have a made-up app with a test at the end. The test pushes a message to a queue, and checks that a counter was incremented. There are also some ongoing events that will be happening unrelated to test (for example, cloud.Schedule pushes a message "bloop" to the queue every 5 seconds). This app is leveraging the distributed nature of the cloud - even though pushing to queue causes processor to be invoked, there might not necessarily be a single request ID that which connects the events together. What logs would make sense to be printed from this example?

a) "success!"
b) "pushing: floop", "success!"
c) "pushing: floop", "processing: floop", "success!"
d) "pushing: bloop", "processing: bloop", "pushing: floop", "processing: floop", "success!"

More logs is usually better, but I could also imagine it being confusing to see unrelated logs mixed in with your test logs. It's sort of like if you create a child process in your computer, you may have to decide whether to inherit its STDIN/STDOUT or not. (except in the cloud, not all processes have parents? 🤔) @tsuf239 I'm curious if you have any thoughts

@tsuf239
Copy link
Contributor Author

tsuf239 commented Aug 10, 2023

I believe that the cloud targets should behave like the sim
(and it may not be such a hard thing to implement if we declare a log group at the start of the run of the app, and make sure to log everything there (then filter out aws meta logs))
I run this on the sim and this is the output:

pass ┌ logging.wsim » root/env0/test:message is processed
          │ pushing: floop
          │ processing: floop
          └ success!

What we can do in the future, when people start to have too many logs, is to specify the resource name:

pass ┌ logging.wsim » root/env0/test:message is processed
          │ pusher:
                  pushing: floop
          │ schedule:
                  processing: floop
          └ test:
                  success!

Ordered by the timestamp,
This can be a really cool feature of the preview environment too- we can store/pass more information per line (location in wing code, scope, etc... )

@tsuf239 tsuf239 moved this from 🏗 In progress to 🤝 Backlog - handoff to owners in Wing Aug 14, 2023
@tsuf239 tsuf239 removed their assignment Aug 28, 2023
@github-actions
Copy link

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

@github-actions github-actions bot added the Stale label Oct 10, 2023
@staycoolcall911 staycoolcall911 added needs-discussion Further discussion is needed prior to impl and removed Stale labels Oct 10, 2023
Copy link

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

@staycoolcall911
Copy link
Contributor

This works now Wing 0.51.26.
Closing.
image

@github-project-automation github-project-automation bot moved this from 🤝 Backlog - handoff to owners to ✅ Done in Wing Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working needs-discussion Further discussion is needed prior to impl 🎨 sdk SDK
Projects
Archived in project
Development

No branches or pull requests

3 participants