adding grpc-gateway to generate bff reporting definitions

[bdomen-ggl]

This is to create the proto messages for gRPC-Gateway. gRPC-Gateway uses a Go gateway, so these messages will be converted to Go. The Kotlin messages will be added later when setting up the gRPC service (not the gateway).

Right now the BFF proto messages are exactly the same as the Reporting service v2alpha messages. This will surely change, but we're still working out how the UI will look and not sure how to make those messages yet. So for now, just going to use the existing proto definitions to put something here.

[wfa-reviewable]

This change is 

[SanjayVas]

Reviewed 2 of 7 files at r1, all commit messages.
Reviewable status: 2 of 7 files reviewed, 5 unresolved discussions (waiting on @bdomen-ggl)

---

build/reporting_ui/reporting_ui_deps.bzl line 20 at r1 (raw file):

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive", "http_file")

def reporting_ui_repositories():

The existing pattern is for external workspaces, but there is no external reporting_ui workspace. I think these should just go under wfa_measurement_system_repositories

Code quote:

reporting_ui_repositories

---

src/main/proto/wfa/measurement/reporting/bff/BUILD.bazel line 12 at r1 (raw file):

GO_IMPORT_PATH = "wfa/measurement/reporting/bff/reportingpb"

go_proto_compiler(

The compilers should go somewhere under build/. Maybe build/grpc_gateaway.

---

src/main/proto/wfa/measurement/reporting/bff/BUILD.bazel line 40 at r1 (raw file):

go_proto_compiler(
    name = "grpc_gateway_proto_compiler",

nit: Maybe rename for consistency?

Suggestion:

go_grpc_gateway

---

src/main/proto/wfa/measurement/reporting/bff/BUILD.bazel line 125 at r1 (raw file):

    importpath = GO_IMPORT_PATH,
    protos = [":metric_proto"],
    deps = [

I may not be understanding go_proto_compiler properly, but my I thought that the deps we add there are the library deps needed by any go_proto_library that uses that compiler plugin. If that's the case, these can just be added to the go_proto_compiler deps.

Also consider adding a go_grpc_gateway_proto_library macro that wraps go_proto_library with the appropriate compilers.

---

src/main/proto/wfa/measurement/reporting/bff/metric.proto line 1 at r1 (raw file):

// Copyright 2023 The Cross-Media Measurement Authors

Public APIs should be versioned, so put all of these in a v1alpha subpackage.

[bdomen-ggl]

@SanjayVas Ready for re-review.

Reviewable status: 0 of 11 files reviewed, 4 unresolved discussions (waiting on @SanjayVas)

---

build/reporting_ui/reporting_ui_deps.bzl line 20 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

The existing pattern is for external workspaces, but there is no external reporting_ui workspace. I think these should just go under wfa_measurement_system_repositories

Done.

---

src/main/proto/wfa/measurement/reporting/bff/BUILD.bazel line 12 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

The compilers should go somewhere under build/. Maybe build/grpc_gateaway.

Done.

---

src/main/proto/wfa/measurement/reporting/bff/BUILD.bazel line 125 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

I may not be understanding go_proto_compiler properly, but my I thought that the deps we add there are the library deps needed by any go_proto_library that uses that compiler plugin. If that's the case, these can just be added to the go_proto_compiler deps.

Also consider adding a go_grpc_gateway_proto_library macro that wraps go_proto_library with the appropriate compilers.

Ah got it. I was mostly copying from the gprc-gateway code/examples. I updated both of these.

---

src/main/proto/wfa/measurement/reporting/bff/metric.proto line 1 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Public APIs should be versioned, so put all of these in a v1alpha subpackage.

Done.

[SanjayVas]

Reviewed 11 of 11 files at r2, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @bdomen-ggl)

---

build/repositories.bzl line 110 at r2 (raw file):

    http_archive(
        name = "com_github_grpc_ecosystem_grpc_gateway",

When importing Bazel repos use the name in that repo's WORKSPACE file.

Suggestion:

grpc_ecosystem_grpc_gateway

---

build/grpc_gateway/defs.bzl line 19 at r2 (raw file):

load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")

def go_grpc_gateway_proto_library(name, import_path, protos, embed = []):

See https://bazel.build/extending/macros#conventions

Suggestion:

visibility = None, **kwargs

---

src/main/proto/wfa/measurement/reporting/bff/v1alpha/BUILD.bazel line 66 at r2 (raw file):

)

go_proto_library(

If these all need the grpc-gateway protoc plugin then they should use the macro.

Code quote:

go_proto_library

---

src/main/proto/wfa/measurement/reporting/bff/v1alpha/BUILD.bazel line 112 at r2 (raw file):

)

# go_proto_library(

Accidentally left this in?

---

src/main/proto/wfa/measurement/reporting/bff/v1alpha/metric.proto line 191 at r2 (raw file):

message Metric {
  option (google.api.resource) = {
    type: "reporting.halo-cmm.org/Metric"

These should have a different type URL to make it clear that they are not the same as the Reporting API messages (even though they are currently copies).

Suggestion:

ui.reporting.halo-cmm.org/Metric

[bdomen-ggl]

Reviewable status: 4 of 11 files reviewed, 5 unresolved discussions (waiting on @SanjayVas)

---

build/repositories.bzl line 110 at r2 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

When importing Bazel repos use the name in that repo's WORKSPACE file.

Ah got it. Copied this from others who imported grpc-gateway.

---

build/grpc_gateway/defs.bzl line 19 at r2 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

See https://bazel.build/extending/macros#conventions

Thanks, saw that doc, but the kwargs part wasn't terribly clear.

---

src/main/proto/wfa/measurement/reporting/bff/v1alpha/BUILD.bazel line 66 at r2 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

If these all need the grpc-gateway protoc plugin then they should use the macro.

You're right. I was thinking this morning I forgot to finish converting them yesterday and I didn't. Got distracted by lint failures and forgot to come back.

---

src/main/proto/wfa/measurement/reporting/bff/v1alpha/BUILD.bazel line 112 at r2 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Accidentally left this in?

Yes, part of the forgot to finish converting them.

---

src/main/proto/wfa/measurement/reporting/bff/v1alpha/metric.proto line 191 at r2 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

These should have a different type URL to make it clear that they are not the same as the Reporting API messages (even though they are currently copies).

ok

[SanjayVas]

Reviewed 7 of 7 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @bdomen-ggl)

---

build/grpc_gateway/defs.bzl line 19 at r2 (raw file):

Previously, bdomen-ggl wrote…

Thanks, saw that doc, but the kwargs part wasn't terribly clear.

It comes from Python, which Starlark is loosely based on.

[stevenwarejones]

Reviewed 4 of 11 files at r2, 7 of 7 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Marco-Premier)

[bdomen-ggl]

Reviewed 4 of 11 files at r2, 6 of 7 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @bdomen-ggl)