dbt-labs / metricflow

MetricFlow allows you to define, build, and maintain metrics in code.
https://docs.getdbt.com/docs/build/about-metricflow
Other
1.13k stars 94 forks source link

[Bug] `meta` field not added to the metric #1365

Open saurabh0402 opened 1 month ago

saurabh0402 commented 1 month ago

Is this a new bug in metricflow?

Current Behavior

Scavaging through the documentation, we found two ways of adding metadata for metrics

We tried both syntaxes. But when dbt compile is run, neither the semantic_manifest.json nor the output.json has the value added to the metadata field. As a result, running metricFlowEngine.list_metrics() also does not return any metadata.

Expected Behavior

The value should be added to the metadata field. So, the semantic_manifest.json should have the following.

{
      "name": "new_customers",
      ...
      "metadata": {
            "feature": "customers",
      },
      "label": "New Customer"
    }

And metricFlowEngine.list_metrics() should return the value in metadata field as well.

Steps To Reproduce

  1. Create a metric with the meta field
    metrics:
    - name: new_customers
    type: SIMPLE
    type_params:
      measure: new_customers
    label: "New Customer"
    meta:
      feature: customers
  2. Run dbt compile and then check the semantic_manifest.json or run metricFlowEngine.list_metrics().
  3. The output won't have any value in metadata field.

Relevant log output

No response

Environment

- OS: MacOS
- Python: 3.11.3
- dbt: 1.8.3
- metricflow: 0.7.1

Which database are you using?

snowflake

Additional Context

No response

saurabh0402 commented 1 month ago

Hey @tlento, can you please check this one? I can try picking this up if it's a bug. If not, please let me know how I can get the metadata field to work?

tlento commented 3 weeks ago

MetricFlow itself has no notion of a meta field, and as such it won't appear in the semantic_manifest.json. You can see that the protocol we use for metrics doesn't include it.

The metadata field you see in semantic_manifest.json is actually an internal construct for allowing us to produce pointers to locations in the user-defined semantic manifest configuration for where validation errors are discovered. I'm not fundamentally opposed to putting the dbt meta in there but I think it's probably better if we figure out a way to more cleanly separate user-land arbitrary key/value pairs from system-supplied entries. I'm pretty sure there was an issue open to support this, but it hasn't been a priority for us.

As for the dbt output, the dbt core parser should be taking the raw meta field parsed out of the YAML and passing it straight into the metric object it includes in the manifest, which should in turn be serialized inside of the manifest.json.

If this isn't happening that suggests a bug in the core parser.

tlento commented 3 weeks ago

I think your options are:

  1. Rely on the manifest.json output for the meta attributes you care about. If they aren't appearing that is a bug, and you should open an issue in dbt-core so we can get it fixed.
  2. Add user-meta to the metadata property in the dbt-semantic-interfaces protocols.
  3. Add a new property so we can differentiate between user-metadata and system-metadata, either by moving the system info to some other property or by just appending a user-metadata type property