dapr / go-sdk Goto Github PK

There are new Proto files after multi state stores support , which causes breaking changes in state API.

So need to update Proto files from Dapr runtime for gRPC client in go-sdk

All the gRPC API invocation on Dapr are already wrapped in authContext method so if the DAPR_API_TOKEN env var is defined, the go client will already use it to populate the outgoing context. This issue is about allowing more explicit way if propagating the auth token on client creation.

Is your feature request related to a problem? Please describe.

In repo dapr/dapr, the version of protoc is v3.1.4.0.

In repo dapr/go-sdk, the version of protoc is v3.11.2.

I have to switch protoc version for these two projects (and also for java sdk)

Describe the solution you'd like

In all the repo, we should use the same version of protoc. Since v3.14.0 is uesd in dapr/dapr, we need to upgrade other repo to this version.

I think support http header is very important in real world like my project

Describe the solution you'd like
Almost go router design do not return error, suggest to AddxxxHandler cancels error return, avoid many error deal, may panic when handler is invalid.

Additional context

Refer Design
net/http HandleFunc
Gin Router
fasthttp recommand third-party routers

I do not know too much about dapr design. If I'm wrong, please just ignore it.

State APIs should support sending metadata in the request.

Dapr runtime issue Reference : dapr/dapr#1909

Describe the bug
Check if sdk needs a change to distinguish between etag mismatch and other state api issues

To Reproduce
Error code internal is returned when state api fails due to etag mismatch error

Expected behavior
dapr runtime changes the error code returned by the state apis for etag mismatch related errors and other errors. Check if the sdk needs a change. dapr/dapr#2647

Currently, the Go SDK has 2 methods to publish events:

• PublishEvent
• PublishEventfromCustomContent (also note the F should have been uppercased here)

The latter doesn't really do much more than the former, besides calling json.Marshal internally and setting the ContentType to application/json.

With #173, I've proposed adding 2 more variants for those methods: PublishEventWithMetadata and PublishEventfromCustomContentWithMetadata (F is lowercased here for consistency). These address #164 and also allow doing things like setting the TTL.

If #173 is merged (which I hope it is!) we will have 4 methods.

Yet, there will still be things that can't be done: setting options that are already available in the protobuf definition, but can't be set using the higher-level PublishEvent* APIs. One example includes setting the ContentType of the document (it's set to text/plain for the "regular" methods and application/json for the "fromCustomContent" methods). In the future, we could expect that there will be other things that can't be set using the higher level APIs.

I am proposing that the PublishEvent* APIs be rationalized:

1. Ideally we would have less than 4 methods
2. It should be possible to have access to a lower-level function that allows passing the protobuf object directly or a similar object (such as SaveBulkState for saving state)

For example, it could be:

• Deprecate PublishEventfromCustomContentWithMetadata and remove it in version 2.0
• Change PublishEvent's last parameter to be interface{}:
  • If the argument is a []byte or string, it will maintain the current behavior
  • If it's a protobuf object or something similar to what SaveBulkState uses, then use that object which allows setting all options.
  • If it's something else, it will be JSON-ified like PublishEventfromCustomContentWithMetadata does today (and set content type to application/json)

This will remain backwards-compatible today (PublishEventfromCustomContentWithMetadata can print a log message that it's deprecated but can continue to work until v2.0).

I am happy to propose a PR if you accept this suggestion, or discuss it further.

Validate the ability to pass existing context with parent trace ID as an input into the client calls. Ensure the ID is propagated into the client invocation against the Dapr API

In what area(s)?

/area test-and-release

Describe the feature

Enable PR gating on unit test coverage to ensure we do not regress on test coverage. For now the agreed upon rule is that no PR should reduce the test coverage. We may expand these rules later.

Part of dapr/dapr#2226

Release Note

RELEASE NOTE: ADD Added codecov rules to ensure that no PR reduces unit test coverage.

Is your feature request related to a problem? Please describe.

In the GRPCClient struct, protoClient is the property that contains the lower-level gRPC client: https://github.com/dapr/go-sdk/blob/main/client/client.go#L165

This is currently a private property, not exposed publicly. There are situations where applications may benefit from being able to interact with this object directly, having access to the lower-level gRPC APIs.

Describe the solution you'd like

Create a public getter method that returns the protoClient object so external objects can use it.

Describe alternatives you've considered

There are 2 alternatives for making low-level calls, but they're both sub-par:

1. Using the Dapr HTTP APIs which don't require access to the gRPC object, but these are potentially slower and require fiddling with the correct request/response structs manually
2. Creating a new gRPC connection, but it's less efficient than reusing the existing one.

Describe the proposal

This is to update protobuf client along with dapr/dapr#1409

In what area(s)?

/area test-and-release

Ask your question here

When will go-sdk release actor example?
I already study dotnet-sdk and java-sdk,But I still don't know what to write actor in go-sdk?

Is there any plan to support actor feature in go-sdk? thank you

As part of dapr/dapr#2226 we need to perform a unit test coverage audit for this package. The purpose of this audit is to identify unit test coverage gaps that we would like to close ahead of v1.0.

Audit Guidelines:

1. We're not shooting for a particular coverage percentage, rather we're looking to add tests which deliver on ensuring correctness and reliability. We prefer efforts that deliver coverage of critical components, rather than just bumping up our coverage percentage.
2. Please create issues for each identified high value area. Be sure to add the label "area/test/unit" and be sure to link this issue and dapr/dapr#2226 in the issue discussion. Also, be sure to add them to the appropriate rc1 and 1.0 milestones.
3. You can close this ticket and update the table in dapr/dapr#2226 once a maintainer of this package has signed off on the list of high value test coverage tickets.
4. Unit testing will always be an ongoing process. The goal of this audit is not to be "done" with unit test coverage, just to ensure that we have a reasonable baseline of coverage that we can maintain and improve upon ahead of the v1.0 release.

Upgrade Golang Lint to 1.30.0.
Add it to build workflow.

Update the SDK for the following breaking changes:

1. Service invocation now handles queryparams as string and not Map of string->string. Get the latest proto and fix any code in the SDK that refers to that.

2. When content type is "application/octet-stream", CloudEvent's data will now be in data_base64 and that should be exposed as byte[] to the application.

Is your feature request related to a problem? Please describe.

I can't delete a state when request level metadata is required.

Describe the solution you'd like

In current api definition of client interface, there are to method about delete:

type Client interface {
	// DeleteState deletes content from store using default state options.
	DeleteState(ctx context.Context, store, key string) error

	// DeleteStateWithETag deletes content from store using provided state options and etag.
	DeleteStateWithETag(ctx context.Context, store, key, etag string, opts *StateOptions) error
}

suggest to add metadata parameter in state delete api:

type Client interface {
	// keep this simple one no change
	DeleteState(ctx context.Context, store, key string) error

	// change this one, add meta
	// DeleteStateWithETag(ctx context.Context, store, key, etag string, opts *StateOptions) error
       DeleteStateWithETag(ctx context.Context, store, key, etag string, meta map[string]string, opts *StateOptions) error
}

Similar to what we do in issue Adding support for metadata in State API Requests #1965

Describe alternatives you've considered

Additional context

Remove retry options for state apis once the issue in runtime is implemented dapr/dapr#1861

If we want to ensure that there's only one gRPC internal client being created we can wrap the internal init logic with Once.Do.

https://golang.org/pkg/sync/#Once.Do

Does the SDK version (currently at v0.7.1) correspond to the Dapr version (currently at v0.8)? Should it?

Update SDK protos to the latest version from dapr/dapr

Is your feature request related to a problem? Please describe.
When I add service invocation handler, I want to deal some same operations. So I want to add the middleware support. Maybe you can think about it.

Describe the solution you'd like
Maybe like gin middleware achieve, maybe need many same web frameware function achieve in the go-sdk.

Just an advice, Thanks!

I find that there is only an example code in the readme.md. wondering that if there is a contribution chance for me to add a pub/sub example to go-sdk.

Describe the proposal

Update proto files for 0.8.0 release

Currently the go SDK supports client side only. As in app invoking Dapr. There is a lot of gRPC plumbing that could be simplify to help users create Dapr serving apps. This proposal is to create new package in the SDK called server to go along the current client package and provide that functionality.

Example developer experience with the gRPC server (similar server implementation will be required for HTTP)

import (
	"log"
	daprd "github.com/dapr/go-sdk/server/grpc"
)

func main() {
	// create a Dapr service server
	server, err := daprd.NewServer(50001)
	if err != nil {
		log.Fatalf("failed to start the server: %v", err)
	}

	// add some invocation handlers
	server.AddInvokeHandler("EchoMethod", echoHandler)

	// add some topic subscriptions
	server.AddTopicSubscription("messages", messageHandler)

	// start the server
	if err := server.Start(); err != nil {
		log.Fatalf("server error: %v", err)
	}
}

func echoHandler(contentTypeIn string, dataIn []byte) (contentTypeOut string, dataOut []byte) {
	content := string(dataIn)
	log.Printf("content: %s", content)
	return "text/plain; charset=UTF-8", []byte(content)
}

func messageHandler(event *daprd.TopicEvent) error {
	log.Printf("event - Topic:%s, ID:%s, Data: %s", event.Topic, event.ID, string(event.Data))
	return nil
}

Is your feature request related to a problem? Please describe.
I cannot set the partition key for Kafka without metadata support in PublishEvent API .

Describe the solution you'd like
Add extra argument "metadata" of type map[string]string to this API and so that it can be set in PublishEventRequest at https://github.com/dapr/go-sdk/blob/main/client/pubsub.go#L20

Describe the bug
When I followed the example in the example to practice the invoke part, I found that I could not add the "--app-port" parameter to the command that launchs the client.

To Reproduce

1. launch the serving

   dapr run --app-id serving \
            --app-protocol grpc \
            --app-port 50001 \
            --dapr-grpc-port 3500 \
            --log-level debug \
            --components-path ./config \
            go run ./serving/grpc/main.go
   

2. launch the client with --app-port 50002

   dapr run --app-id caller \
            --app-port 50002 \
            --components-path ./config \
            --log-level debug \
            go run ./client/main.go
   

3. see the error

   == APP == panic: error invoking service: rpc error: code = Internal desc = invoke API is not ready
   

Expected behavior
Here's what it looks like in the dapr list if don't add --app-port:

~# dapr list

  APP ID  HTTP PORT  GRPC PORT  APP PORT  COMMAND               AGE  CREATED              PID
  server  38515      3500       50001     go run ./serving/...  6s   2021-06-07 23:07.47  11877
  caller  36071      33433      0         go run ./client/m...  5s   2021-06-07 23:07.48  11990

I'm not sure if this is a bug, but I'd like to know if it's necessary to add --app-port？

Thx.

I've added some basic tests for input params and pb.* to local type converters. The scaffolding for client target is in place but for some reason the Dapr types are not reconciled at runtime. Once resolved, we will need to add additional tests (line 19 in client/client_test.go)

https://github.com/github/renaming

This is as part of this parent epic.

Document in examples to be explicit to reuse DaprClient, creating multiple DaprClients will lead to port exhaustions.

This is tracking issue in dapr/dapr. Need to support that capability when it lands in the Dapr API. This is breaking change so critical the go-sdk capability lands at the same time.

Describe the proposal

Hey folks! Saw this stuff was released recently and am super excited about it!

One thing I noticed while looking through these SDKs is that they seem to just be exposing the generated gRPC code of the desired language. I was wondering if it's worth wrapping the gRPC logic in some thin, language specific wrappers. This could allow language specific patterns to be exposed in each of the SDKs rather than expecting all consumers to understand gRPC right out of the gate.

I threw a branch together of something that it could look like, but wanted to get some feedback on the idea from the maintainers before I go down a rabbit hole.

master...bign8:language-specific-api

Looking forward to collaborating!

(Also, sorry if this comes across and edgy or short, I have yet to have my morning coffee ☕️)

Describe the proposal

Update auto-generated go sdk for changes in proto files(multi store feature)

In the current HTTP serving implementation the subscriber hander response is not generating the newly added status response which results in incorrect handling in Dapr

dapr install in namespace myns, appid=myapp, and http handle router /test
forward port on local:

kubectl port-forward svc/myapp 50001:50001 3500:3500 8080:8080 -n myns

invoke in go sdk:

 res, err := client.InvokeService(ctx, "myapp.myns", "test")

this result is : error invoking service: rpc error: code = NotFound desc = Not Found

browser open or call it use curl: http://localhost:3500/v1.0/invoke/myapp.myns/method/test it has correct result

ps: Is there any other way to call the service locally

dapr init and success.
Then go get github.com/dapr/go-sdk.

and I find it in go/pkg/mod/github.com/dapr/go-sdk and run client.

client abser$ dapr run --app-id client --protocol grpc --app-port 4000 go run main.go

then I got an error for I don't have Redis yaml

❌  open /Users/abser/go/pkg/mod/github.com/dapr/[email protected]/example/client/components/redis.yaml: no such file or directory

Is your feature request related to a problem? Please describe.

I do not see any lint for this, enable lint makes project better I think.

Describe the solution you'd like

Introduce golangci-lint.

I have done at local, If possible, I will post it.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

In addition to the current []byte as an input type, provide a way to pass raw structs

With the addition of the Bulk Get API to the State Building Block, this also needs to be added to the different SDKs.

See API reference here: https://github.com/dapr/docs/blob/master/reference/api/state_api.md#get-bulk-state

I'm trying to communicate with a dapr publisher and a MassTrasnit consumer and coming across some interoperability issues when using the .NET SDK. The communication from the sidecar to the broker always passes content type of text/plain which is causing issues. I think it would help with interoperability if we could specify that header.

I was looking at https://github.com/dapr/components-contrib/blob/master/pubsub/rabbitmq/rabbitmq.go#L179 and noticed content type is hardcoded to text/plain.

I'm assuming when I'm using the for example a pubsub.rabbitmq component using the .NET SDK and publishing a message using the client implementation (GRPC client), it ends up publishing to the actual broker/topic with text plain as the content type.

This makes it difficult for interoperability with other frameworks like MassTransit which requires a specific content type of application/vnd.masstransit+json (https://masstransit-project.com/architecture/interoperability.html).

Is there any remedy or thoughts for integrating with different consumers other than Dapr or do we need to be 'all in'?

Describe the solution you'd like
Be able to configure the content-type so then the .NET SDK can also configure the content-type.

Describe alternatives you've considered
I could convert the MassTransit consumer to use Dapr.

I was able to work around this by implementing a customer message deserializer in MassTransit.

Describe the bug

Creating a client and server in Go, I need to return a proper error status code.

My handlers look like these:

daprSrv.AddServiceInvocationHandler("new", func(ctx context.Context, in *common.InvocationEvent) (out *common.Content, err error) {
	if ... {
		return nil, status.Error(codes.InvalidArgument, "Invalid content type")
	}
	return res, nil
})

The problem is that the errors are wrapped multiple times, both in the server and in the client. Because of that, it looks impossible to retrieve the status code.

On the client-side, the error is wrapped multiple times, including on

On the server-side, OnInvoke method wraps the error here:

It's possible to unwrap the error as wrapped on the client side with a code like:

for {
	unwrapped := errors.Unwrap(err)
	if unwrapped == nil {
		break
	}
	err = unwrapped
}

grpcErr, ok := status.FromError(err)
if !ok {
	// Not a gRPC error
	return
}

However because the error is wrapped in the server too, it's not a gRPC error object anymore, so on the client grpcErr has always code = Unknown and I can't get the status code.

Expected behavior

gRPC errors in the client and server should not be wrapped so the original status code and message are preserved. Otherwise, all errors arrive with status code Unknown and it's not possible for the client to understand what kind of error it is.

Describe the bug

After running make protos, the code in branch master can't compile.

$ make test
go mod tidy
go test -count=1 \
                -race \
                -coverprofile=coverage.txt \
                -covermode=atomic \
                ./...
# github.com/dapr/go-sdk/service/grpc
service/grpc/invoke.go:41:18: cannot use in.HttpExtension.Querystring (type string) as type map[string]string in assignment
# github.com/dapr/go-sdk/client
client/invoke.go:51:60: cannot use queryMap (type map[string]string) as type string in field value
# github.com/dapr/go-sdk/service/grpc [github.com/dapr/go-sdk/service/grpc.test]
service/grpc/invoke.go:41: cannot use in.HttpExtension.Querystring (type string) as type map[string]string in assignment
# github.com/dapr/go-sdk/client [github.com/dapr/go-sdk/client.test]
client/invoke.go:51: cannot use queryMap (type map[string]string) as type string in field value
client/invoke_test.go:122:39: non-integer string index "foo"
client/invoke_test.go:123:50: non-integer string index "url"
FAIL    github.com/dapr/go-sdk/client [build failed]
?       github.com/dapr/go-sdk/dapr/proto/common/v1     [no test files]
?       github.com/dapr/go-sdk/dapr/proto/runtime/v1    [no test files]
FAIL    github.com/dapr/go-sdk/service/grpc [build failed]
ok      github.com/dapr/go-sdk/service/http     0.036s  coverage: 82.5% of statements

To Reproduce

1. run make protos
2. run make test

Expected behavior
The code should compile and pass test.

This is the actions from review of this issue "Consistency of API names surfaced by each SDK" dapr/dapr#2535

Service Invocation API

• Rename invokeService to invokeMethod
• Rename param serviceID to appId
• Rename param method to methodName

Like this
func (c *GRPCClient) invokeMethod(ctx context.Context, appID, methodName string) (out []byte, err error)
func (c *GRPCClient) InvokeMethodWithContent(ctx context.Context, appID, methodName string, content *DataContent) (out []byte, err error)

State Store API

• Rename param from store to storeName
• Rename param from data to value

For example like this on one of the APIs
func (c *GRPCClient) SaveState(ctx context.Context, storeName, key string, value []byte) error

Secret Store API

• Rename param from store to storeName

For example like this
func (c *GRPCClient) GetSecret(ctx context.Context, storeName, key string, meta map[string]string) (out map[string]string, err error)

Binding

• Change struct BindingInvocation to InvokeBindingRequest
• Make sure that in BindingInvocation struct uses name, operation, data as names.

Pubsub

• Rename param from component to pubsubName
• Rename param from topic to topicName
• Rename param from in to data

For example like this
func (c *GRPCClient) PublishEvent(ctx context.Context, pubsubName, topicName string, data []byte) error

Is your feature request related to a problem? Please describe.
I am trying to use this sdk for some integration tests for dapr. The server, however, doesn't allow for the IP address of the server to be discoverable. In many build environments using hardcoded ports can lead to conflicts and discovering unused ports can lead to race conditions.

Describe the solution you'd like
I want the http.Server or grpc.Server to expose the address that is used by the implicit listener (if the address passed in was ":0", for example.

or

I want to be able to pass in a net.Listener to the http or grpc server so I can control the address allocation apriori.

Describe alternatives you've considered
I've tried discovering an unused port, closing the listener, and passing the address of that listener. The code is cumbersome and doesn't follow conventions used by the standard library or other projects.

Describe the bug
PublishEventfromCustomContent serializes the input payload to JSON and stuffs it into gRPC request. However it doesn't set the DataContentType field. Dapr later will default it to "text/plain" during event consumption - this can cause confusions.

Is this intended?

To Reproduce
Call PublishEventfromCustomContent to publish an event and examine the data content type.

Expected behavior
If the payload is JSON serialized the DataContentType should be set accordingly - "application/json" in this case.

Having a dependency in the SDK might create conflicts with apps that depend on a different version of the same package. This is a problem where a dependency is still unstable (v0.x) and can have breaking changes in minor or patch upgrades.

The action item is to review the dependencies in the SDK, specially unstable versions and confirm those are required. Ideally, removing unstable dependencies.

The new HTTP service contractor takes only address right now. In apps that combine Dapr with other HTTP handlers it would be nice to allow creation of Dapr HTTP service (callback) with existing HTTP mux.