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.

Sign up for GitHub

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

add extension version #2754

Closed
wants to merge 1 commit into from
Closed

add extension version #2754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion cmd/ui.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

"gopkg.in/yaml.v3"

"go.k6.io/k6/js/modules"
"go.k6.io/k6/lib"
"go.k6.io/k6/lib/consts"
"go.k6.io/k6/output"
Expand Down Expand Up @@ -81,7 +82,13 @@ func getColor(noColor bool, attributes ...color.Attribute) *color.Color {

func getBanner(noColor bool) string {
c := getColor(noColor, color.FgCyan)
return c.Sprint(consts.Banner())
banner := make([]string, 0, len(modules.GetJSModuleVersions())+1)

banner = append(banner, consts.Banner())
for pkg, version := range modules.GetJSModuleVersions() {
banner = append(banner, fmt.Sprintf("extension %s %s", pkg, version))
}
return c.Sprint(strings.Join(banner, "\n"))
Comment on lines -84 to +91
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this output needs to be a bit more visually pleasing. Here's what it currently looks like:
2022-11-09-184448_928x410_scrot

The extension information shouldn't be part of the banner. The banner itself is only reserved for... well, the k6.io banner 😄

Instead, if we do want to include this information as part of the k6 run output in addition to the k6 version output (I do think it makes sense to have it in both places), then let's integrate it into the section below the banner, as suggested in #1741.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

}

func printBanner(gs *globalState) {
Expand Down
4 changes: 4 additions & 0 deletions cmd/version.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/spf13/cobra"

"go.k6.io/k6/js/modules"
"go.k6.io/k6/lib/consts"
)

Expand All @@ -16,6 +17,9 @@ func getCmdVersion(globalState *globalState) *cobra.Command {
Long: `Show the application version and exit.`,
Run: func(_ *cobra.Command, _ []string) {
printToStdout(globalState, fmt.Sprintf("k6 v%s\n", consts.FullVersion()))
for path, version := range modules.GetJSModuleVersions() {
printToStdout(globalState, fmt.Sprintf("extension %s %s\n", path, version))
}
Comment on lines +20 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

k6 supports both JS and output extensions. So we should make sure that both are shown in this output (same goes for the k6 run output).

You can test by building a binary with an output extension, e.g.:

XK6_K6_REPO=github.com/HarrisChu/k6 xk6 build master \
  --with github.com/vesoft-inc/k6-plugin \
  --with github.com/grafana/xk6-output-influxdb

It should be a matter of doing something similar as in js/modules/modules.go and calling getPackageVersion(mod) from output.RegisterExtension(). You'll probably want to move getPackageVersion() somewhere where it's accessible by both (maybe lib/extensions.go?).

},
}
}
52 changes: 50 additions & 2 deletions js/modules/modules.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package modules
import (
"context"
"fmt"
"reflect"
"runtime/debug"
"strings"
"sync"

Expand All @@ -15,8 +17,9 @@ const extPrefix string = "k6/x/"

//nolint:gochecknoglobals
var (
modules = make(map[string]interface{})
mx sync.RWMutex
modules = make(map[string]interface{})
moduleVersions = make(map[string]string)
Comment on lines +20 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to reuse the modules map, and have its values be a small struct like:

type module struct {
	mod     interface{}
	version string
}

This way getPackageVersion() would only return the version, as its name implies, and not also set (another) global map value.

This will complicate things slightly, as you'll need to change wherever GetJSModules() is called (or change GetJSModules() itself to continue to return just the modules without the version; though I think the first approach is slightly better), but it should avoid the repetition, and be a bit cleaner in the end. It would also avoid the need for a separate GetJSModuleVersions() function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks good

mx sync.RWMutex
)

// Register the given mod as an external JavaScript module that can be imported
Expand All @@ -34,6 +37,38 @@ func Register(name string, mod interface{}) {
panic(fmt.Sprintf("module already registered: %s", name))
}
modules[name] = mod
getPackageVersion(mod)
}

func getPackageVersion(mod interface{}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func getPackageVersion(mod interface{}) {
func getModuleVersion(mod interface{}) {

I think this is slightly clearer, as we are getting the Go module version after all (and the map is called moduleVersions).

Copy link
Contributor Author

@HarrisChu HarrisChu Nov 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

good catch.

btw, If a package has two modules, it just has one package in map.
how about display the module name? e.g.

 extensions: github.com/dgzlopes/k6-extension-notifications k6/x/ext1 v1.0.0
             github.com/dgzlopes/k6-extension-notifications k6/x/ext2 v1.0.0

t := reflect.TypeOf(mod)
p := t.PkgPath()
if p == "" {
if t.Kind() != reflect.Ptr {
return
}
if t.Elem() != nil {
p = t.Elem().PkgPath()
}
}
buildInfo, ok := debug.ReadBuildInfo()
if !ok {
return
}
for _, dep := range buildInfo.Deps {
packagePath := strings.TrimSpace(dep.Path)
if strings.HasPrefix(p, packagePath) {
if _, ok := moduleVersions[packagePath]; ok {
return
}
if dep.Replace != nil {
moduleVersions[packagePath] = dep.Replace.Version
} else {
moduleVersions[packagePath] = dep.Version
}
break
}
}
}

// Module is the interface js modules should implement in order to get access to the VU
Expand All @@ -56,6 +91,19 @@ func GetJSModules() map[string]interface{} {
return result
}

// GetJSModuleVersions returns a map of all registered js modules package and their versions
func GetJSModuleVersions() map[string]string {
mx.Lock()
defer mx.Unlock()
result := make(map[string]string, len(moduleVersions))

for name, version := range moduleVersions {
result[name] = version
}

return result
}

// Instance is what a module needs to return
type Instance interface {
Exports() Exports
Expand Down