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

go-autorest depends on contrib.go.opencensus.io/exporter/ocagent v0.3.0, which has a bug #439

Closed
v-jaswel opened this issue Aug 6, 2019 · 6 comments

Comments

@v-jaswel
Copy link

v-jaswel commented Aug 6, 2019

go-autorest dependent on contrib.go.opencensus.io/exporter/ocagent v0.3.0, which has a bug

Bug Report

I'm trying to write a sample for the Bing Spellcheck service using the Azure SDK for Go.
When I run my code, I get the error:

# Spell_Go/vendor/contrib.go.opencensus.io/exporter/ocagent
vendor\contrib.go.opencensus.io\exporter\ocagent\ocagent.go:216:35: cannot convert true (type untyped bool) to type "Spell_Go/vendor/github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1".ConstantSampler_ConstantDecision
vendor\contrib.go.opencensus.io\exporter\ocagent\ocagent.go:216:35: invalid operation: csamp.Decision == true (mismatched types "Spell_Go/vendor/github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1".ConstantSampler_ConstantDecision and bool)

I believe this is because go-autorest depends on contrib.go.opencensus.io/exporter/ocagent v0.3.0.

https://github.com/census-ecosystem/opencensus-go-exporter-ocagent/blob/v0.3.0/ocagent.go, line 216, is:

alwaysSample := csamp.Decision == true

contrib.go.opencensus.io/exporter/ocagent v0.3.0 in turn depends on github.com/census-instrumentation/opencensus-proto v0.2.1, which as of 20190806 is the current release.

https://github.com/census-instrumentation/opencensus-proto/blob/master/gen-go/trace/v1/trace_config.pb.go, lines 27-33, are:

type ConstantSampler_ConstantDecision int32

const (
	ConstantSampler_ALWAYS_OFF    ConstantSampler_ConstantDecision = 0
	ConstantSampler_ALWAYS_ON     ConstantSampler_ConstantDecision = 1
	ConstantSampler_ALWAYS_PARENT ConstantSampler_ConstantDecision = 2
)

So the line in ocagent.go should instead be:

alwaysSample := csamp.Decision == tracepb.ConstantSampler_ALWAYS_ON

Making that change in my local vendor folder fixes the issue, and indeed they've done the same thing in the current release (v0.6.0) of contrib.go.opencensus.io/exporter/ocagent - see https://github.com/census-ecosystem/opencensus-go-exporter-ocagent/blob/master/ocagent.go line 323.

This might have some relation to issue #438.

Other information (adapted from Azure SDK for Go bug report template) follows.

  • import path of package in question, e.g. .../services/compute/mgmt/2018-06-01/compute
"github.com/Azure/go-autorest/autorest"
  • SDK version e.g. master, latest, 18.1.0
    • Specify the exact commit if possible; one way to get this is the REVISION
      column output by dep status "github.com/Azure/azure-sdk-for-go.
PROJECT                                             CONSTRAINT     VERSION        REVISION  LATEST   PKGS USED
contrib.go.opencensus.io/exporter/ocagent           v0.3.0         v0.3.0         7c99379   v0.3.0   1
github.com/Azure/azure-sdk-for-go                   ^32.1.0        v32.1.0        a629ae7   v32.1.0  2
github.com/Azure/go-autorest                        ^12.4.1        v12.4.1        ba1147d   v12.4.1  6
github.com/census-instrumentation/opencensus-proto  v0.2.1         v0.2.1         d89fa54   v0.2.1   4
github.com/dgrijalva/jwt-go                         v3.2.0         v3.2.0         06ea103   v3.2.0   1
github.com/golang/protobuf                          v1.3.2         v1.3.2         6c65a55   v1.3.2   12
github.com/grpc-ecosystem/grpc-gateway              v1.9.5         v1.9.5         ad529a4   v1.9.5   3
github.com/hashicorp/golang-lru                     v0.5.3         v0.5.3         7f827b3   v0.5.3   1
go.opencensus.io                                    v0.21.0        v0.21.0        df6e200   v0.21.0  17
golang.org/x/net                                    branch master  branch master  ca1201d   ca1201d  6
golang.org/x/sync                                   branch master  branch master  1122301   1122301  1
golang.org/x/sys                                    branch master  branch master  51ab0e2   51ab0e2  1
golang.org/x/text                                   v0.3.2         v0.3.2         342b2e1   v0.3.2   16
google.golang.org/api                               v0.7.0         v0.7.0         02490b9   v0.7.0   1
google.golang.org/genproto                          branch master  branch master  fa694d8   fa694d8  3
google.golang.org/grpc                              v1.22.1        v1.22.1        045159a   v1.22.1  33
  • output of go version
go version go1.12.6 windows/amd64
  • What happened?

Described above.

  • What did you expect or want to happen?

Expected output:

Token 1:
  Offset: 5
  Token: gtaes
  Type: UnknownToken
    Suggestion 1: gates
    Score: 1.000000
  • How can we reproduce it?
  1. Create environment variable SPELLCHECK_SUBSCRIPTION_KEY and set it to a valid Bing Spellcheck subscription key.
  2. Create environment variable SPELLCHECK_ENDPOINT and set it the endpoint that corresponds to your subscription key.
  3. Copy the following code into a file named spell.go.
package main

import (
	"context"
	"fmt"
	"github.com/Azure/azure-sdk-for-go/services/cognitiveservices/v1.0/spellcheck"
	"github.com/Azure/go-autorest/autorest"
	"log"
	"os"
)

func main() {
	if "" == os.Getenv("SPELLCHECK_SUBSCRIPTION_KEY") {
		log.Fatal("Please set/export the environment variable SPELLCHECK_SUBSCRIPTION_KEY.")
	}
	var subscription_key string = os.Getenv("SPELLCHECK_SUBSCRIPTION_KEY")
	if "" == os.Getenv("SPELLCHECK_ENDPOINT") {
		log.Fatal("Please set/export the environment variable SPELLCHECK_ENDPOINT.")
	}
	var endpoint string = os.Getenv("SPELLCHECK_ENDPOINT")

	// Get the context, which is required by the SDK methods.
	ctx := context.Background()

	client := spellcheck.New()
	// Set the subscription key on the client.
	client.Authorizer = autorest.NewCognitiveServicesAuthorizer(subscription_key)
	client.Endpoint = endpoint

	result, err := client.SpellCheckerMethod (ctx, "bill gtaes", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "spell", "", "")
	if nil != err {
		log.Fatal(err)
	}

	tokens := *result.FlaggedTokens
	for i, token := range tokens {
		fmt.Printf ("Token %d:\n", i + 1)
		fmt.Printf ("  Offset: %d\n  Token: %s\n  Type: %s\n", *token.Offset, *token.Token, token.Type)
		for j, suggestion := range *token.Suggestions {
			fmt.Printf ("    Suggestion %d: %s\n", j + 1, *suggestion.Suggestion)
			fmt.Printf ("    Score: %f\n", *suggestion.Score)
		}
		fmt.Println()
	}
}
  1. Run dep init.
  2. Run go run spell.go.
  • Anything we should know about your environment.

Windows 10 Enterprise 18.03
Go environment described above.

Please let me know if I can provide any additional information. Thank you!

@jhendrixMSFT
Copy link
Member

In my fix for #438 my hope was to align the dependencies between the go mod and dep consumers. Unfortunately this doesn't work for dep due to lack of support for constraints on transitive dependencies, see golang/dep#1124.

@jhendrixMSFT
Copy link
Member

My current POR is to revert the changes to Gopkg.toml/Gopkg.lock, I just need to confirm this isn't going to make things worse for kubernetes.

@v-jaswel
Copy link
Author

Hello and thank you for your reply. Unfortunately it seems contrib.go.opencensus.io/exporter/ocagent@v0.4.6 still has the same issue.

I now get:

# <path>/vendor/contrib.go.opencensus.io/exporter/ocagent
vendor\contrib.go.opencensus.io\exporter\ocagent\ocagent.go:297:35: cannot convert true (type untyped bool) to type "<path>/vendor/github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1".ConstantSampler_ConstantDecision
vendor\contrib.go.opencensus.io\exporter\ocagent\ocagent.go:297:35: invalid operation: csamp.Decision == true (mismatched types "<path>/vendor/github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1".ConstantSampler_ConstantDecision and bool)
vendor\contrib.go.opencensus.io\exporter\ocagent\transform_stats_to_metrics.go:53:3: unknown field 'Descriptor_' in struct literal of type "<path>/vendor/github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1".Metric
vendor\contrib.go.opencensus.io\exporter\ocagent\transform_stats_to_metrics.go:59:45: undefined: "<path>/vendor/github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1".Metric_MetricDescriptor
vendor\contrib.go.opencensus.io\exporter\ocagent\transform_stats_to_metrics.go:67:11: undefined: "<path>/vendor/github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1".Metric_MetricDescriptor

As the error output indicates they're still doing the same thing in ocagent.go:

https://github.com/census-ecosystem/opencensus-go-exporter-ocagent/blob/v0.4.6/ocagent.go#L297

alwaysSample := csamp.Decision == true

@jhendrixMSFT
Copy link
Member

This has been fixed in go-autorest@v13.0.0. The SDK will be updated with this version at the end of this month. Until then you can use an override stanza to use v13.

[[override]]
  name = "github.com/Azure/go-autorest"
  version = "13.0.0"

@v-jaswel
Copy link
Author

Thank you very much! I hope to have a sec to try it out tomorrow but as I see you're using ocagent 0.6.0 that should indeed fix it.

@wiazur
Copy link

wiazur commented Dec 7, 2019

I tested our sample in Go and the issue looks fixed!
https://github.com/Azure-Samples/cognitive-services-quickstart-code/blob/master/go/BingSpellCheck/BingSpellCheckQuickstart.go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants