SDI-2115: remove metric.String() in plugin-lib-go

[candysmurf]

Summary of changes:

• added local method toString(ns []string) for GRPC mock test.
• change tests use the local toString instead of from plugin-lib-go.

Testing done:

• test medium
• test small

@intelsdi-x/snap-maintainers

[IzabellaRaulin]

@candysmurf, why metric.String() has been removed from plugin-lib-go? Should it has both methods String() and Strings() as snap's core.Namespace? Could you explain the motivation of removing this?

I prefer to avoid creating private "toString" function in each package if it's possible - regarding https://github.com/intelsdi-x/snap-plugin-lib-go/pull/38.

[candysmurf]

@IzabellaRaulin, thanks for your concern. Please refer to this issue intelsdi-x/snap-plugin-lib-go#34 opened in plugin-lib-go. That's the decision made in our backlog grooming.
Feel free to express your concerns.

[IRCody]

@IzabellaRaulin: I think the goal is to remove the implied guarantee that lives behind offering a String() method. With the work that has gone into arbitrary separators now the implementation behind a String() method would be decently complex and subject to future change. We don't want to provide a guarantee that it will work in a specific way.

[kindermoumoute]

@candysmurf, as for the plugin lib PR, I suggest you replace string "ShouldEqual" by slice "ShouldResemble":

So(slice1, ShouldResemble, slice2)

Doing this avoid creating a "toString" function as @IzabellaRaulin mentioned it.

[candysmurf]

@IRCody & @kindermoumoute, if not comparing strings but array of strings, it requires comparison of each array element or using reflect.DeepEqual(). An additional function "stringsEqual" will need to be added. Please confirm you prefer "stringEqual" over "toString". Should we run a benchmark? :-)

[candysmurf]

@IRCody, I can see the good use of doing array comparison if we like to recommend this way to all plugins and if it has no performance penalty, I would be glad to change it.

[candysmurf]

@IRCody, thanks for doing the benchmark! We agreed to not use toString() method for two reasons:

• our code often does the conversion first, such as strings.Join(ns, "/"), then comparison. It's less performant than comparing the slices directly.
• we promote using namespace array value or []string slice directly instead of converting []string -> string, then compare.

[IRCody]

LGTM

[kindermoumoute]

Why not to write a slice of string directly?
plugin.NewNamespace("intel", "mock", "host0", "baz").Strings() ==> []string{"intel", "mock", "host0", "baz"}

[candysmurf]

not sure why it was like that. Probably make sure the correct library is used. Not core.NewNamespace

[kindermoumoute]

LGTM