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

feat: add telemetry to Node.js and TypeScript function templates #719

Merged
merged 6 commits into from
Dec 15, 2021

Conversation

lance
Copy link
Member

@lance lance commented Dec 13, 2021

Changes

🎁 This commit pulls in the latest version of faas-js-runtime which supports metrics exposed at /metrics. Also, concidentally, adds CloudEvent batch support, so Node.js and TypeScript functions may be invoked with a set of more than one event simultaneously.

UPDATE: This PR also includes some changes to Makefile in order to remove any stray go.sum files that end up in the go templates. And it removes a stray go.sum file that ended up in the go templates.

/kind enhancement

Fixes: #710

Signed-off-by: Lance Ball lball@redhat.com

This commit pulls in the latest version of faas-js-runtime which supports
metrics exposed at /metrics. Also, concidentally, adds CloudEvent batch
support, so Node.js and TypeScript functions may be invoked with a set of
more than one event simultaneously.

Fixes: knative#710

Signed-off-by: Lance Ball <lball@redhat.com>
@knative-prow-robot knative-prow-robot added kind/enhancement size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 13, 2021
Signed-off-by: Lance Ball <lball@redhat.com>
Signed-off-by: Lance Ball <lball@redhat.com>
@lance
Copy link
Member Author

lance commented Dec 13, 2021

I'm not sure how best to address the code style issue with pkged.go

Signed-off-by: Lance Ball <lball@redhat.com>
@codecov
Copy link

codecov bot commented Dec 13, 2021

Codecov Report

Merging #719 (2448555) into main (6c67be0) will increase coverage by 0.54%.
The diff coverage is 42.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #719      +/-   ##
==========================================
+ Coverage   39.93%   40.48%   +0.54%     
==========================================
  Files          42       42              
  Lines        3986     3999      +13     
==========================================
+ Hits         1592     1619      +27     
+ Misses       2172     2150      -22     
- Partials      222      230       +8     
Impacted Files Coverage Δ
cmd/run.go 61.53% <38.46%> (+21.15%) ⬆️
docker/runner.go 8.23% <100.00%> (+8.23%) ⬆️
cmd/root.go 60.18% <0.00%> (+0.92%) ⬆️

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 b190b52...2448555. Read the comment docs.

@knative-prow-robot knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 13, 2021
Copy link
Contributor

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

/approve

@knative-prow-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lance, zroubalik

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

context: Context,
cloudevent?: CloudEvent
cloudevent?: CloudEvent<string> | CloudEvent<Customer>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know TypeScript, I am just curious: would cloudevent?: CloudEvent<string | Customer> work?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually it would. Nice catch.

@@ -13,20 +13,17 @@ test('Unit: handles a valid event', (t) => {
customerId: '01234'
};
// A valid event includes id, type and source at a minimum.
const cloudevent: CloudEvent = new CloudEvent({
const cloudevent: CloudEvent<Customer> = new CloudEvent<Customer>({
id: '01234',
type: 'com.example.cloudevents.test',
source: '/test',
data
Copy link
Contributor

Choose a reason for hiding this comment

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

why this is not data: data?

Copy link
Member Author

Choose a reason for hiding this comment

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

@lkingland
Copy link
Member

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 15, 2021
@knative-prow-robot knative-prow-robot merged commit d7cfe6e into knative:main Dec 15, 2021
jrangelramos pushed a commit to jrangelramos/func that referenced this pull request Oct 30, 2023
Signed-off-by: Matej Vasek <mvasek@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/enhancement lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Function telemetry and metrics for Node.js and TypeScript
5 participants