Skip to content

Conversation

@dubee
Copy link
Member

@dubee dubee commented Aug 29, 2018

Description

Currently after invoking a blocking request, a user must call activation get with the corresponding activation ID to see associated logs. Changes here improve the development experience by optionally allowing a user to retrieve the last 4KB of user logs for blocking invocations.

Related issue and scope

  • I opened an issue to propose and discuss this change (#????)

My changes affect the following components

  • API
  • Controller
  • Message Bus (e.g., Kafka)
  • Loadbalancer
  • Invoker
  • Intrinsic actions (e.g., sequences, conductors)
  • Data stores (e.g., CouchDB)
  • Tests
  • Deployment
  • CLI
  • General tooling
  • Documentation

Types of changes

  • Bug fix (generally a non-breaking change which closes an issue).
  • Enhancement or new feature (adds new functionality).
  • Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

  • I signed an Apache CLA.
  • I reviewed the style guides and followed the recommendations (Travis CI will check :).
  • I added tests to cover my changes.
  • My changes require further changes to the documentation.
  • I updated the documentation where necessary.

@dubee dubee added the wip label Aug 29, 2018

def getTruncatedLogs(logs: ActivationLogs) = {
var totalLogSize = 0
val maxLogSize = 1024 * 4
Copy link
Member Author

Choose a reason for hiding this comment

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

Should be configurable per deployment.

@rabbah
Copy link
Member

rabbah commented Aug 29, 2018

this will hold up the active ack (and hence slow down activations) until some logs are available --- what's the criteria for getting logs to attach to the activation/active ack to avoid slowing down activations?

how do you indicate that the activation should have logs or not (ie when you wait for logs and when you don't)?

also this seems like something that can be solved entirely on the client side (cli) - why make backend changes for this?

@rabbah
Copy link
Member

rabbah commented Aug 29, 2018

FWIW i think features like this should have issues and discussions on the dev list to iron out details and understand the implications before seeing the code.

@dubee
Copy link
Member Author

dubee commented Aug 29, 2018

@rabbah, there is an optional query parameter for the invocation API that will allow logs to be shown. During development of actions, the developer should not care how fast actions execute if he or she is concerned with seeing logs from blocking invocations.

@codecov-io
Copy link

Codecov Report

Merging #3994 into master will increase coverage by 4.67%.
The diff coverage is 67.39%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3994      +/-   ##
==========================================
+ Coverage   76.34%   81.01%   +4.67%     
==========================================
  Files         146      146              
  Lines        7084     7107      +23     
  Branches      440      439       -1     
==========================================
+ Hits         5408     5758     +350     
+ Misses       1676     1349     -327
Impacted Files Coverage Δ
.../src/main/scala/whisk/core/connector/Message.scala 35.29% <100%> (+9.8%) ⬆️
...hisk/core/controller/actions/SequenceActions.scala 94.49% <100%> (+12.84%) ⬆️
.../main/scala/whisk/core/controller/WebActions.scala 91.15% <100%> (+3.46%) ⬆️
...src/main/scala/whisk/core/controller/Actions.scala 93% <100%> (+1.54%) ⬆️
...isk/core/controller/actions/PrimitiveActions.scala 89.62% <100%> (+1.48%) ⬆️
...a/whisk/core/loadBalancer/InvokerSupervision.scala 95.62% <100%> (+11.06%) ⬆️
...core/controller/actions/PostActionActivation.scala 100% <100%> (ø) ⬆️
...ain/scala/whisk/core/invoker/InvokerReactive.scala 67.64% <26.31%> (+67.64%) ⬆️
...cala/whisk/core/containerpool/ContainerProxy.scala 93.44% <87.5%> (+1.3%) ⬆️
...core/database/cosmosdb/RxObservableImplicits.scala 0% <0%> (-100%) ⬇️
... and 51 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b0224b...5bb5772. Read the comment docs.

@apache apache deleted a comment from markusthoemmes Sep 4, 2018
@apache apache deleted a comment from rabbah Sep 4, 2018
@markusthoemmes
Copy link
Contributor

This has been sitting in WIP, got stale and is inactive since 2 months. Any update?

@dubee dubee closed this Jan 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants