MethodDescriptor.CustomOptions no longer works in 3.11.0-rc2 (when old generated code is used)

[JamesNK]

I want to get custom options off a MethodDescriptor. Specifically I want to get HttpRule which is from Google.Api.CommonProtos 1.7.0. In Google.Protobuf 3.8.0 I can successfully get this option by doing this:

const int HttpRuleFieldId = 72295728;
var success = methodDescriptor.CustomOptions.TryGetMessage<HttpRule>(HttpRuleFieldId, out httpRule);

With Google.Protobuf 3.9.0 and 3.10.0 CustomOptions.TryGetMessage<T> errors, I believe related to this issue: #6824

When I updated to 3.11.0-rc2 CustomOptions.TryGetMessage<T> no longer errors, but it also doesn't return a value. You should fix that for backwards compatibility.

I see that CustomOptions has been made obsolete. How do I use the new API?

const int HttpRuleFieldId = 72295728;
const uint TagId = 0; // what is this?

var extension = new Extension<Google.Protobuf.Reflection.MethodOptions, HttpRule>(HttpRuleFieldId, FieldCodec.ForMessage<HttpRule>(TagId, HttpRule.Parser));
var httpRule = methodDescriptor.GetOption<HttpRule>(extension); // returns null

[ObsidianMinor]

In the new API, extension identifiers are generated automatically in container classes like AnnotationsExtensions for annotations.proto. So, to use the new API, add a using statement for Google.Api and access the extension through GetOption(AnnotationsExtensions.Http)

using Google.Api;

var extension = AnnotationsExtensions.Http;
var rule = methodDescriptor.GetOption(extension);

The issue with the extension not being discovered is due to an issue I believe in the GetAllExtensions method used during the start of reflection to make sure all extensions are properly registered. I'll see if I can get it fixed in #6938 or if I need to move the fix to another PR.

[ObsidianMinor]

Actually, wait. Are you loading descriptors dynamically in this case @JamesNK?

[JamesNK]

I don't know what it means to get descriptors dynamically or not.

I have this proto file:

syntax = "proto3";

import "google/api/annotations.proto";

package greet;

// The greeting service definition.
service Greeter {
  // Sends a greeting
  rpc SayHello (HelloRequest) returns (HelloReply) {
    option (google.api.http) = {
      get: "v1/greeter/{name}"
    };
  }
}

// The request message containing the user's name.
message HelloRequest {
  string name = 1;
  string test_name = 2;
}

// The response message containing the greetings
message HelloReply {
  string message = 1;
}

I reference it in my csproj with <Protobuf Include="greet.proto" GrpcServices="Server" />, and I'm using Grpc.Tools 2.25.0.

I have a reference Google.Api.CommonProtos, required for generated by Greeter service that has added AnnotationsReflection.Descriptor as a FileDescriptor, and for HttpRule type:

descriptor = pbr::FileDescriptor.FromGeneratedCode(descriptorData,
    new pbr::FileDescriptor[] { global::Google.Api.AnnotationsReflection.Descriptor, },
    new pbr::GeneratedClrTypeInfo(null, new pbr::GeneratedClrTypeInfo[] {
      new pbr::GeneratedClrTypeInfo(typeof(global::Greet.HelloRequest), global::Greet.HelloRequest.Parser, new[]{ "Name", "TestName" }, null, null, null),
      new pbr::GeneratedClrTypeInfo(typeof(global::Greet.HelloReply), global::Greet.HelloReply.Parser, new[]{ "Message" }, null, null, null)
    }));

And then I am getting the HttpRule descriptor from the method descriptor.

[JamesNK]

using Google.Api;

var extension = AnnotationsExtensions.Http;
var rule = methodDescriptor.GetOption(extension);

I don't have Google.Api.AnnotationsExtensions.Http in my solution. And there are no results for it in the repo that Google.Api.CommonProtos is from: https://github.com/googleapis/gax-dotnet/search?q=AnnotationsExtensions&unscoped_q=AnnotationsExtensions

[ObsidianMinor]

Oh I see. In this case you're mixing generated code from different versions. The code from gax-dotnet is using another previous version of code-gen while you're using a future version Google.Protobuf that's expecting you to have extension identifiers generated.

[ObsidianMinor]

I keep forgetting, is this kind of setup supported @jtattermusch?

[JamesNK]

Would a solution be to generate code from annotations.proto and http.proto myself, with the type visibility internal (can you do that? It would avoid type conflicts with anyone who is using Google.Api.CommonProtos) and include that code in my project?

[ObsidianMinor]

Yes that'd work. All you need is the internal_access csharp option when generating code.

[JamesNK]

Hmm, but it won't work because Google.Api.AnnotationsReflection.Descriptor needs to be public because it is referenced as a dependency by the generated service.

And making the types public could easy create conflicts with anyone who is referencing Google.Api.CommonProtos.

You either need to do:

1. Update Google.Api.CommonProtos to have new generated code
2. Make CustomOptions.TryGetMessage<T> work again in 3.11.0
3. Support GetOption with mixed generated code versions

Right now I don't see a way for this to work.

[ObsidianMinor]

I'll probably just fix CustomOptions for instances like this, so GetOption will only work with code that already has extension identifiers. We can include it in the next patch release with #6938 and #6674 and any other issues that come up in the meantime.

[JamesNK]

When will the next patch release be?

[ObsidianMinor]

No idea, you'd have to ask @rafi-kamal.

[rafi-kamal]

We are going to do a new release as soon as all the RC2 issues are fixed. I think this one and #6936 are the only one that need to be fixed.

[JamesNK]

What's the state of this issue? I tested the latest package - 3.11.2 - and it is still broken.

[rafi-kamal]

Looks like the fix didn't make into the final release. Unfortunately we don't have any ETA for the next release at this moment.

[ObsidianMinor]

All I need to finish #7005 up is write some sort of tests for it. I'm going to write a quick and easy version using a slightly modified version of unittest_custom_options_proto3.proto and later I think I can come back and overhaul tests a bit further to make sure issues like this are caught ahead of time in the future.

[daniel-munch-cko]

#6936 isn't released yet either. (Just taking advantage of it being mentioned in this issue earlier)

[zeinali0]

Would be any release soon to fix this issue?

[vchirikov]

@JamesNK that's why https://github.com/aspnet/AspLabs/tree/master/src/GrpcHttpApi doesn't work now?

Edited:
ok, it works, but it looks like you forgot to add information about Google.Api.CommonProtos to instructions.

[aesalazar]

HI all. I have been watching this issue for the past few months as we are rely on the custom options. Is there any update?

[jtattermusch]

HI all. I have been watching this issue for the past few months as we are rely on the custom options. Is there any update?

Actually, I think CustomOptions would just work fine for you if you use the latest version or both the generated code and the runtime - AFAIK the problem in this issue was that CustomOptions didn't work if you had a new protobuf runtime, but an old version of the generated code. (The unmerged PR #7005 actually attempts to fix only the scenario where you have old generated code an a new runtime - everything else should work).

Also, you can use the extensions API instead of relying on CustomOptions.

[jtattermusch]

I just closed the long standing PR #7005 which was an attempt to fix this.
This is only a problem that exhibits itself when a new runtime is used with old generated code (pre 3.11.)

• the 3.11 release is quite old now and there's been several releases since
• in the meantime, there's been significant changes to the generated code (the new Span-based serialization and deserialization), so regenerating the generated code is really recommended regardless of this issue (even though it's strictly speaking not necessary).

[aesalazar]

Disregard the message below. RTFM. I missed the part about recompiling the proto files. Works great now!

Hi all. Sorry, have not been able to revsist this. So, how would I get to the `enum_pretty_name` attributes here:

enum WorldRegionCode {
    UNDEFINED_REGION    = 0 [(protocommon.enum_pretty_name) = "{undefined}"];
    NORTH_AMERICA       = 1 [(protocommon.enum_pretty_name) = "North Americ"];
    LATIN_AMERICA       = 2 [(protocommon.enum_pretty_name) = "Latin America"];
    EUROPE              = 3 [(protocommon.enum_pretty_name) = "Europe"];
    ASIA                = 4 [(protocommon.enum_pretty_name) = "Asia"];
    AUSTRALIA           = 5 [(protocommon.enum_pretty_name) = "Australia"];
}

Where the protocommon.proto contains:

extend google.protobuf.EnumValueOptions {
  string enum_pretty_name = 50002;
}

Prior to the changes here, we could do this:

//Get the enum descriptor by type
var enumDescriptor = MyProtoReflection.Descriptor.EnumTypes.FirstOrDefault(eDesc => eDesc.ClrType == typeof(TEnum));
var dict = new Dictionary<TEnum, string>();

//Need to extract the attributes in the most convoluted way imaginable
foreach (var edv in enumDescriptor.Values)
{
    //See if there is a custom value;
    var customValue = edv.CustomOptions.TryGetString(EnumPrettyNameField, out var value)
        ? value
        : default;

    var enumValue = (TEnum)Enum.ToObject(typeof(TEnum), edv.Number);
    dict[enumValue] = customValue;
}

but even in the latest version of the library we do not see it anywhere in the compiled FileDescriptor. I debugged it and all CustomOptions settings were empty.

Thanks.
Ernie

[ObsidianMinor]

With new generated code, there should be an extension identifier named ProtocommonExtensions.EnumPrettyName. Using that you can call the new extension API like so:

var customValue = edv.GetOptions()?.GetExtension(ProtocommonExtensions.EnumPrettyName) ?? default;

[aesalazar]

@ObsidianMinor Thanks! Thats a much cleaner way to do it. Appreciate the help!

Ernie